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.

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

ReferenceError: name is not defined #5401

Closed
3 tasks done
alexblack opened this issue Jul 10, 2022 · 8 comments · Fixed by #5404
Closed
3 tasks done

ReferenceError: name is not defined #5401

alexblack opened this issue Jul 10, 2022 · 8 comments · Fixed by #5404

Comments

@alexblack
Copy link

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/remix

SDK Version

7.6.0

Framework Version

7.6.0

Link to Sentry event

https://sentry.io/organizations/syncwith/issues/3415220654/?project=5880203&query=is%3Aunresolved

Steps to Reproduce

I am trying out sentry-remix for the first time, and now when my app encounters a 404 it seems like our 404 page doesn't get used, and I get an unexpected exception tracked to sentry

The error seems to be in:

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

Screen Shot 2022-07-10 at 11 43 03 AM

Screen Shot 2022-07-10 at 11 43 32 AM

My app in root looks like:

function App() {
  const data = useLoaderData<RootLoaderData>();
  return (
    <Document data={data}>
      <Outlet />
      <footer></footer>
    </Document>
  );
}

export default withSentry(App);

Expected Result

I expected our normal 404 page to show, I think through the CatchBoundary

Actual Result

Screen Shot 2022-07-10 at 11 43 03 AM

Screen Shot 2022-07-10 at 11 43 32 AM

Error: name is not defined
File "/Users/alex/dev/micro/webapp/node_modules/@sentry/remix/cjs/utils/instrumentServer.js", line 65, col 40, in
captureRemixServerException(err, name);
File "/Users/alex/dev/micro/webapp/node_modules/@remix-run/server-runtime/server.js", line 393, col 18, in handleDocumentRequest
return await handleDocumentRequest(request.clone(), responseStatusCode, responseHeaders, entryContext);
File "/Users/alex/dev/micro/webapp/node_modules/@remix-run/server-runtime/server.js", line 49, col 18, in requestHandler
response = await handleDocumentRequest({
File "/Users/alex/dev/micro/webapp/node_modules/@sentry/remix/cjs/utils/instrumentServer.js", line 134, col 16, in
var res = (await origRequestHandler.call(this, request, loadContext)) ;
File "/Users/alex/dev/micro/webapp/node_modules/@remix-run/express/server.js", line 41, col 22, in
let response = await handleRequest(request, loadContext);

@alexblack
Copy link
Author

I might be misunderstanding, but I don't see what name refers to here, it seems like this would fail to compile... wouldn't it?

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

function makeWrappedDocumentRequestFunction(
  origDocumentRequestFunction: HandleDocumentRequestFunction,
): HandleDocumentRequestFunction {
  return async function (
    this: unknown,
    request: Request,
    responseStatusCode: number,
    responseHeaders: Headers,
    context: Record<symbol, unknown>,
  ): Promise<Response> {
    let res: Response;

    const activeTransaction = getActiveTransaction();
    const currentScope = getCurrentHub().getScope();

    if (!activeTransaction || !currentScope) {
      return origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context);
    }

    try {
      const span = activeTransaction.startChild({
        op: 'remix.server.documentRequest',
        description: activeTransaction.name,
        tags: {
          method: request.method,
          url: request.url,
        },
      });

      res = await origDocumentRequestFunction.call(this, request, responseStatusCode, responseHeaders, context);

      span.finish();
    } catch (err) {
      captureRemixServerException(err, name);
      throw err;
    }

    return res;
  };
}

@onurtemizkan
Copy link
Collaborator

Thanks for reporting this @alexblack, will look into why TS allowed that.

@alexblack
Copy link
Author

Great!

@alexblack
Copy link
Author

Do you know if/when a new release will be made with this fix? Thanks!!

@AbhiPrasad
Copy link
Member

@alexblack We have a couple more remix fixes to make, and then we'll make another release! End of week by latest.

@alexblack
Copy link
Author

@AbhiPrasad ok thanks, I reverted to 7.5.1 to avoid this issue of sentry throwing errors on 404 pages

@AbhiPrasad
Copy link
Member

We've just released the fixes in 7.7.0! Thank you for your patience @alexblack!

@alexblack
Copy link
Author

Great to hear, thanks for letting me know

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants