Remix SDK invariant is not captured as Issue

[moishinetzer]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/remix

SDK Version

7.5.0

Framework Version

7.5.0

Link to Sentry event

https://sentry.io/organizations/netzerorg/performance/trace/7a8e8c962aa242aabb565917ff0c891c/

Steps to Reproduce

import invariant from "tiny-invariant";

export const loader: LoaderFunction = async ({ params }) => {
  const user = params["*"];
  invariant(user, "user is required");
  const user= await getUser({ user: user});
  return json({
    user,
  });
};

Now, navigate to a route where the user does not exist.

Expected Result

Invariant is triggered and thrown and an issue is created in sentry dashboard

Actual Result

Invariant is triggered and thrown, however sentry only receives a transaction and doesn't flag it as an issue.

[moishinetzer]

Note: As a sidepoint I am also getting every 10 seconds an actual issue with the following information:

Sentry event: issues/3402471265 (Same org as specified in issue)

/myapp/node_modules/@remix-run/server-runtime/data.js

Object.captureException
{"size":0}

76.   try {
77.    result = await loader({
78.      request: stripDataParam(stripIndexParam(request)),
79.      context: loadContext,
80.      params: match.params
81.    });
82.  } catch (error) {
83.    if (!responses.isResponse(error)) {
84.      throw error; 

[AbhiPrasad]

Hey thanks for writing in. @onurtemizkan mind taking a look when you get some time to help triage this?

[moishinetzer]

My project is now seeing several issues. feel free to reach out to me or look around on my account to see them. None of them have been caused by actual user related issues and seem to be the SDK.

[AbhiPrasad]

There's some RangeError: Maximum call stack size exceeded which we should look at.

RangeError: Maximum call stack size exceeded
  File "/myapp/node_modules/@sentry/src/misc.ts", line 143, col 1, in Object.addExceptionMechanism
  File "/myapp/node_modules/@sentry/remix/cjs/utils/instrumentServer.js", line 45, col 17, in <anonymous>
  File "/myapp/node_modules/@sentry/src/scope.ts", line 515, col 1, in <anonymous>
  File "/myapp/node_modules/@sentry/src/syncpromise.ts", line 58, col 1, in new SyncPromise
  File "/myapp/node_modules/@sentry/src/scope.ts", line 510, col 1, in Scope._notifyEventProcessors
  File "/myapp/node_modules/@sentry/src/scope.ts", line 527, col 1, in <anonymous>
  File "/myapp/node_modules/@sentry/src/syncpromise.ts", line 58, col 1, in new SyncPromise
  File "/myapp/node_modules/@sentry/src/scope.ts", line 510, col 1, in Scope._notifyEventProcessors
  File "/myapp/node_modules/@sentry/src/scope.ts", line 527, col 1, in <anonymous>
  File "/myapp/node_modules/@sentry/src/syncpromise.ts", line 58, col 1, in new SyncPromise

There seems to be problems with adding the event processor, but not sure what's causing it 🤔

[AbhiPrasad]

Perhaps this may help with the above problem.

diff --git a/packages/remix/src/utils/instrumentServer.ts b/packages/remix/src/utils/instrumentServer.ts
index f11eb2186..cb122a79f 100644
--- a/packages/remix/src/utils/instrumentServer.ts
+++ b/packages/remix/src/utils/instrumentServer.ts
@@ -1,4 +1,4 @@
-import { captureException, configureScope, getCurrentHub, startTransaction } from '@sentry/node';
+import { captureException, getCurrentHub, startTransaction } from '@sentry/node';
 import { getActiveTransaction } from '@sentry/tracing';
 import { addExceptionMechanism, fill, loadModule, logger, stripUrlQueryAndFragment } from '@sentry/utils';
 
@@ -98,11 +98,11 @@ function makeWrappedDataFunction(origFn: DataFunction, name: 'action' | 'loader'
   currentScope.setSpan(activeTransaction);
   span.finish();
 } catch (err) {
-      configureScope(scope => {
+      captureException(err, scope => {
     scope.addEventProcessor(event => {
       addExceptionMechanism(event, {
         type: 'instrument',
-            handled: true,
+            handled: false,
         data: {
           function: name,
         },
@@ -110,10 +110,9 @@ function makeWrappedDataFunction(origFn: DataFunction, name: 'action' | 'loader'

       return event;
     });
+        return scope;
   });

-      captureException(err);
-
   // Rethrow for other handlers
   throw err;
 }

[moishinetzer]

Sure. I'll test this out in a few hours and update you then. In the meantime I've added both of you to my private repo if you want to investigate further. Thanks for your help!

[lforst]

There seems to be problems with adding the event processor, but not sure what's causing it 🤔

@AbhiPrasad I investigated similar-looking stack traces here a few days ago. I came to the conclusion that this is most likely due to an event processor being added over and over again (probably on each incoming request).

Looking at our implementation, it might be possible that the same happens here:

sentry-javascript/packages/remix/src/utils/instrumentServer.ts

Lines 101 to 103 in 1cfcb0d

	configureScope(scope => {
	scope.addEventProcessor(event => {
	addExceptionMechanism(event, {

[AbhiPrasad]

Ah, we probably need withScope then!

[moishinetzer]

Still happy to test the above out but not quite sure if thats still the proposed solution?

[onurtemizkan]

@moishinetzer, thanks for the repro case.

About the invariant issue:
I have tested your project, and found the invariant which is failing for business is not from a loader, but from the SSR component.

We are already capturing any errors, including invariants from loader/action functions. But this seems to be another case to be handled inside wrapRequestHandler. I'll open a PR to fix it.

@AbhiPrasad, @lforst: I still can't reproduce the RangeError: Maximum call stack size exceeded locally. Could it be coming from a serverless deployment?

[moishinetzer]

@onurtemizkan Thanks so much for your help!

To confirm, I only started seeing this RangeError issue after it was deployed, so that very well may be the case.

In which case is there any contact where I can email you the link to the deployed site?

It's currently hosted on fly.io free tier using the default indie-stack hosting preconfiguration.

[onurtemizkan]

Thanks @moishinetzer, just sent you an email. 👍

[AbhiPrasad]

I still can't reproduce the RangeError: Maximum call stack size exceeded locally. Could it be coming from a serverless deployment?

Yeah it's probably a serverless issue IMO. Let's try applying the patch in #5362 (comment)?

[moishinetzer]

Sure, I'll go ahead and test that now

[moishinetzer]

After making the above changes (I believe correctly...)

https://sentry.io/organizations/netzerorg/issues/3408515603

I ended up getting the same error however it is now unhandled.

[AbhiPrasad]

Gonna reopen this so we can confirm this is fixed by the above change.

[AbhiPrasad]

@moishinetzer thank you so much for helping us debug this and giving detailed info. We would love to send you some Sentry swag for helping us out, can we reach out through the email in your profile?

[moishinetzer]

@moishinetzer thank you so much for helping us debug this and giving detailed info. We would love to send you some Sentry swag for helping us out, can we reach out through the email in your profile?

Absolutely please do! I have a question for you as well and would love to be in touch! Loving the Sentry team you guys are awesome!

In the meantime I'm going to try out the above change and update you accordingly.

[moishinetzer]

locally the unhandled issue now does not appear. however since I'm not sure how to apply the changes to my deployed version, I haven't been able to check there yet.

[AbhiPrasad]

Absolutely please do! I have a question for you as well and would love to be in touch! Loving the Sentry team you guys are awesome!

Will reach out :)

locally the unhandled issue now does not appear. however since I'm not sure how to apply the changes to my deployed version, I haven't checked there yet.

We'll cut another release of the SDK by end of day eastern timezone, hopefully that'll help you test. If this does fix the unhandled issue - we only have the Maximum call stack size exceeded to investigate. Hopefully that should be solved as well!

[AbhiPrasad]

Here's the release: https://github.com/getsentry/sentry-javascript/releases/tag/7.6.0!

[moishinetzer]

Ok so here's a quick update re my project:

• The maximum call stack issue is resolved!

However for some reason the issue of
https://sentry.io/organizations/netzerorg/issues/3413500624/?query=is%3Aunresolved

Object.captureException:
{"size":0}

has been triggering exactly every 10 seconds.

---

Interesting to note, whenever the invariant issue is triggered as shown in the initial issue, the following shows both when tested locally and deployed.

ReferenceError: name is not defined
    at /myapp/node_modules/@sentry/remix/cjs/utils/instrumentServer.js:65:40
    at handleDocumentRequest (/myapp/node_modules/@remix-run/server-runtime/server.js:400:18)
    at requestHandler (/myapp/node_modules/@remix-run/server-runtime/server.js:49:18)
    at /myapp/node_modules/@sentry/remix/cjs/utils/instrumentServer.js:134:16
    at /myapp/node_modules/@remix-run/express/server.js:39:22

And also whenever I trigger the above error it does not get sent to sentry. Either due to it not being triggered or either due to the fact that this and the above issue are related. I'm inclined to think it's not sent as it does not show in the issue logs when I look for it after triggering it.

On a quick glance this may seem to be a similar issue to #5401

Let me know if there's any other kind of testing I can do to help this issue out!

[moishinetzer]

Super quick update, after further research it seems that #5401 (comment) is definitely what the ReferenceError: name is not defined error is about.

After patching locally not only did the name error go away, the invariant triggers in sentry correctly!

My changes:

    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);
++      captureRemixServerException(err, 'loader');
      throw err;
    }

I'm going to try test this out on the deployed version and update this comment accordingly. Fingers crossed!

Update:

After applying the above change to the deployed version the ReferenceError: name is not defined error does not show and the invariant gets sent correctly as an issue so it's the same result as local!

However the issue mentioned earlier Object.captureException still remains.

[onurtemizkan]

Thanks @moishinetzer,
So, {size: 0} error every 10 seconds might be because of this: https://fly.io/docs/reference/configuration/#services-http_checks.

Looks like another case to handle in the SDK, will get back to you.

[onurtemizkan]

Update:

So, {size: 0} error every 10 seconds might be because of this: https://fly.io/docs/reference/configuration/#services-http_checks.

The source of this one is:

  if (!user) {
    throw redirect("/page");
  }

The redirect response is thrown every 10 seconds from the GET request to root.
Opened a PR: #5405

[moishinetzer]

Just tested out the latest commit and it seems that all problems mentioned in this issue beginning to end have been resolved!

I'll test out the release when it comes out and assuming all's fine I'll close the issue.

Thanks so much @AbhiPrasad @onurtemizkan for all your hard work on this. And ridiculously fast respond time! 👏

[AbhiPrasad]

Fix released with 7.7.0! 🚀

[AbhiPrasad]

@moishinetzer I reached out through email - wanted to confirm if you had gotten it?

[moishinetzer]

@AbhiPrasad yes, received! Just saw it now thanks for the bump!!