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

fix: fetch('').catch() triggers unhandled rejection in debugger #15

Merged
merged 1 commit into from
Jun 7, 2024

Conversation

timfish
Copy link
Contributor

@timfish timfish commented Jun 4, 2024

Due to this v8 debugger issue:
https://issues.chromium.org/issues/41161875

Promise.reject().catch(() => {}) results in the promise rejection still triggering uncaught exception in the debugger even though it's later caught.

This can cause tests to fail when they use the debugger:
getsentry/sentry-javascript#12343

This PR changes the code to use a try/catch block in an async function which doesn't result in the above debugger issue.

@mwenko
Copy link
Contributor

mwenko commented Jun 7, 2024

@djMax seems like you are the one able to review and merge this one.

Could you please take a look and let us know if there is something that blocks merging and releasing the provided solution?

@djMax djMax merged commit e1f97f7 into gas-buddy:main Jun 7, 2024
1 check passed
@timfish timfish deleted the fix/fetch-reject branch June 7, 2024 13:50
@mwenko
Copy link
Contributor

mwenko commented Jun 13, 2024

@djMax
Thanks for merging it 🙏

I just realized that the github action failed to publish the release to npm: https://github.com/gas-buddy/opentelemetry-instrumentation-fetch-node/actions/runs/9417358726/job/25942406732

Could you please take a look?

@djMax
Copy link
Contributor

djMax commented Jun 13, 2024

For some reason it seems like I have to run them twice to get them to work. This is the only repo I have that's like this, but it is reliably doing it. Super weird.

@AbhiPrasad
Copy link

Any updates on a release?

@djMax
Copy link
Contributor

djMax commented Jun 21, 2024

I think it's already out, no?

@AbhiPrasad
Copy link

I only see 1.2.0: https://unpkg.com/browse/opentelemetry-instrumentation-fetch-node@1.2.0/

no GH release updated either.

@mwenko
Copy link
Contributor

mwenko commented Jun 26, 2024

@djMax

Seems like there happened no NPM release.

@AbhiPrasad
Copy link

@djMax sorry for the ping again, but any updates on a release?

@djMax
Copy link
Contributor

djMax commented Jul 8, 2024

I think this is finally out.

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

Successfully merging this pull request may close these issues.

4 participants