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

Unhandled Promise Rejection in debugger on Node startup #12343

Closed
3 tasks done
mwenko opened this issue Jun 4, 2024 · 13 comments · Fixed by #12809 or #13485
Closed
3 tasks done

Unhandled Promise Rejection in debugger on Node startup #12343

mwenko opened this issue Jun 4, 2024 · 13 comments · Fixed by #12809 or #13485
Assignees
Labels

Comments

@mwenko
Copy link

mwenko commented Jun 4, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.7.0

Framework Version

8.7.0

Link to Sentry event

No response

SDK Setup

Sentry.init({
            dsn: dsn,
            environment: environmentWithAppInstance,
            debug: true,
            maxValueLength: maxValueLength,
            release: version,
            integrations: [nodeProfilingIntegration()],
            skipOpenTelemetrySetup: true,
            tracesSampler: (context) => {
                if (context.request?.url?.endsWith("/status")) {
                    return 0;
                }

                return defaultTraceSampleRate;
            },
            profilesSampleRate: profilesSampleRate,
            beforeBreadcrumb(breadcrumb, _) {
                if (!shouldAddBreadcrumb(breadcrumb)) {
                    return null;
                }

                return breadcrumb;
            },

            beforeSend(event: ErrorEvent, hint: Sentry.EventHint) {
                if (!shouldSend(hint)) {
                    return null;
                }

                event = removeAuthorizationHeader(event);

                return event;
            },
        });

Steps to Reproduce

  1. Run nest start --debug --watch

Node: 20.9.0
NestJS: 10.3.8

Expected Result

The application should start without throwing an unhandled promise rejection.

Actual Result

It throws an unhandled promise rejection (see screenshot):

image

If I unmark "uncaught exception" in my editor it starts like expected.

@github-actions github-actions bot added the Package: node Issues related to the Sentry Node SDK label Jun 4, 2024
@timfish
Copy link
Collaborator

timfish commented Jun 4, 2024

This error isn't unhandled as it has a .catch(). You're likely hitting this in the debugger because you have Pause on caught exceptions enabled:

image

@mwenko
Copy link
Author

mwenko commented Jun 4, 2024

This error isn't unhandled as it has a .catch(). You're likely hitting this in the debugger because you have Pause on caught exceptions enabled:

image

Thanks for the quick response.

If you look at the provided screenshot, you'll see that I have Uncaught Exception marked in the breakpoints section. (bottom left corner)

(I noticed that the code has a catch() applied. And it made the issue even weirder to me)

@timfish
Copy link
Collaborator

timfish commented Jun 4, 2024

I think you're seeing this:
microsoft/vscode#116783

Which has a linked v8 dev tools issue which is still open:
https://issues.chromium.org/issues/41161875

The argument is that debugger should not break here because the exception is later caught.

Without the debugger attached, this does not result in an unhandled promise rejection.

@timfish
Copy link
Collaborator

timfish commented Jun 4, 2024

I've opened a PR in opentelemetry-instrumentation-fetch-node which should stop this from hitting the debugger issue.

@timfish timfish self-assigned this Jun 4, 2024
@AbhiPrasad AbhiPrasad added this to the v8 Instrumentation Issues milestone Jun 4, 2024
@timfish timfish changed the title Unhandled Promise Rejection on Node startup Unhandled Promise Rejection in debugger on Node startup Jun 5, 2024
@AbhiPrasad AbhiPrasad self-assigned this Jul 8, 2024
@mwenko
Copy link
Author

mwenko commented Jul 10, 2024

Hmm, I still experiencing this issue (using the latest release 8.17.0).
The debugger stops because of an "unhandled promise rejection".

image

@AbhiPrasad AbhiPrasad reopened this Jul 10, 2024
@AbhiPrasad
Copy link
Member

I don't understand how this could be uncaught when there is a catch block right below it 🤔

Maybe this is because of how fetch internals work?

@mwenko
Copy link
Author

mwenko commented Jul 12, 2024

I've opened a PR in opentelemetry-instrumentation-fetch-node which should stop this from hitting the debugger issue.

@timfish
Did you test this PR if it resolves the issue?

@timfish
Copy link
Collaborator

timfish commented Jul 12, 2024

@timfish
Did you test this PR if it resolves the issue?

No sorry. I looked at the open Chromium debugger issues and the causes and then made assumptions based on that and what I know about how awaiting promises vs then work.

As @AbhiPrasad says above, the changes I made should have fixed this but the fact that it hasn't suggests this rejection is triggered by the fetch internals.

@mwenko
Copy link
Author

mwenko commented Jul 22, 2024

@timfish

I opened a PR where the issue has been resolved in my project (running against 8.19 of Sentry)

gas-buddy/opentelemetry-instrumentation-fetch-node#16

@timfish
Copy link
Collaborator

timfish commented Aug 14, 2024

We're going to drop opentelemetry-instrumentation-fetch-node in favour of @opentelemetry/instrumentation-undici which should help solve this issue. I'll respond in this issue when this makes it into a release.

@mwenko
Copy link
Author

mwenko commented Aug 19, 2024

We're going to drop opentelemetry-instrumentation-fetch-node in favour of @opentelemetry/instrumentation-undici which should help solve this issue. I'll respond in this issue when this makes it into a release.

Thank you for the update.
I think it's a good decision to go with the official package.

@AbhiPrasad
Copy link
Member

We've released https://github.com/getsentry/sentry-javascript/releases/tag/8.30.0 with using the official package.

@mwenko
Copy link
Author

mwenko commented Sep 11, 2024

We've released https://github.com/getsentry/sentry-javascript/releases/tag/8.30.0 with using the official package.

Great work - it works!

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