Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

@sentry/remix createSentryRequestHandler makes deferred data in root loader resolve {} instead of original data. #9235

Closed
3 tasks done
hanayashiki opened this issue Oct 13, 2023 · 2 comments · Fixed by #9242
Closed
3 tasks done

@sentry/remix createSentryRequestHandler makes deferred data in root loader resolve {} instead of original data. #9235

hanayashiki opened this issue Oct 13, 2023 · 2 comments · Fixed by #9242
Assignees
@onurtemizkan
Labels
Package: remix Issues related to the Sentry Remix SDK Type: Bug

Comments

@hanayashiki
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/remix

SDK Version

7.73.0

Framework Version

Remix 2.0.1, React 18.2.0

Link to Sentry event

No response

SDK Setup

Not related

Steps to Reproduce

  1. Setup the custom server.mjs as instructed
const createSentryRequestHandler =
  wrapExpressCreateRequestHandler(createRequestHandler);

app.all("*", createSentryRequestHandler({ build }));
  1. Use defer and promise in root.jsx
import { defer } from "@remix-run/node";
import { Await, Links, Meta, Outlet, Scripts, useLoaderData } from "@remix-run/react";
import { Suspense } from "react";

export const loader = () => {
  return defer({
    deferred: new Promise((r) => setTimeout(() => r(42), 1000)),
  });
};

export default function App() {
  const data = useLoaderData();

  return (
    <html>
      <head>
        <link rel="icon" href="data:image/x-icon;base64,AA" />
        <Meta />
        <Links />
      </head>
      <body>
        <h1>Hello world!</h1>

        <Suspense fallback={'Loading...'}>
          <Await resolve={data.deferred}>
            {(answer) => JSON.stringify(answer)}
          </Await>
        </Suspense>

        <Outlet />

        <Scripts />
      </body>
    </html>
  );
}
  1. Open browser at localhost:3000

Expected Result

Displays 42 on the page.

Actual Result

Displays {} on the page. Check the reproduction repo: https://github.com/hanayashiki/Sentry-Remix-Root-Loader-Deferred-Data-Reproduction
@github-actions github-actions bot added the Package: remix Issues related to the Sentry Remix SDK label Oct 13, 2023
@hanayashiki
Copy link
Author

hanayashiki commented Oct 13, 2023 •
edited
Loading

Just FYI, if the response is a defer(), sentry remix will modify the return data using the following:

https://github.com/getsentry/sentry-javascript/blob/develop/packages/remix/src/utils/instrumentServer.ts#L262

However, in remix DeferredData is not a plain object, so spreading the object here doesn't really make sense.

Tests also don't cover promise values in defer. There should be tests.

For my own case, I patched the code with questions using

      if (isDeferredData(res)) {
        Object.assign(res.data, {
          ...traceAndBaggage,
          remixVersion,
        });
        return res;
      }

Which resolves the case for me.

Other possible solutions:

  1. Just don't use defer.
  2. If you have to use defer in "root" for global access, wrap all routes under one parent route, and use defer in the loader there.

@onurtemizkan onurtemizkan self-assigned this Oct 13, 2023
@onurtemizkan onurtemizkan mentioned this issue Oct 13, 2023
Merged
@AbhiPrasad
Copy link
Member

This was fixed with https://github.com/getsentry/sentry-javascript/releases/tag/7.74.1, please give it a try!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@onurtemizkan onurtemizkan

Labels
Package: remix Issues related to the Sentry Remix SDK Type: Bug
Projects
GitHub Issues with 👀
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(remix): Update defer injection logic. getsentry/sentry-javascript
3 participants
@onurtemizkan @AbhiPrasad @hanayashiki