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

Use minimal data URL for loading fetch() #16

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Conversation

mwenko
Copy link
Contributor

@mwenko mwenko commented Jul 22, 2024

The issue (getsentry/sentry-javascript#12343) that should have been resolved by this PR #15 did not work.

Providing a minimal data URL to Fetch() will not cause an unhandled promise exception and will result in a clean startup. The minimal data URL will also ensure that there is no network activity for the fetch() call, which will not make it depended on any connectivity requirements.

I started my project with Sentry (8.19) with the debugger enabled and it worked.

cc @timfish @djMax

@mwenko
Copy link
Contributor Author

mwenko commented Jul 22, 2024

The issue (getsentry/sentry-javascript#12343) that should have been resolved by this PR #15 did not work.

Providing a Request object will not make the application throw an unhandled Promise exception.

I started my project with Sentry (8.19) with the debugger enabled and it worked.

cc @timfish @djMax

Okay, the current example only works because it fails on the request object creation and will not execute the fetch() at all.

Will watch for other solutions.

@mwenko
Copy link
Contributor Author

mwenko commented Jul 22, 2024

Using now a minimal valid data URL. That URL is valid and will not throw anymore.

It will not involve any network activity, which is also good I think.

@mwenko mwenko changed the title fix: use request object instead of url string Use minimal data URL for loading fetch() Jul 22, 2024
@mwenko
Copy link
Contributor Author

mwenko commented Jul 30, 2024

@djMax Could you please have a review on this one?

@djMax djMax merged commit e44a4c3 into gas-buddy:main Jul 30, 2024
1 check passed
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.

2 participants