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

try/catch fetching the version.json file #9526

Closed
kevinrenskers opened this issue Mar 27, 2023 · 2 comments · Fixed by #9622
Closed

try/catch fetching the version.json file #9526

kevinrenskers opened this issue Mar 27, 2023 · 2 comments · Fixed by #9622
Labels
bug Something isn't working ready to implement please submit PRs for these issues!
Milestone

Comments

@kevinrenskers
Copy link

Describe the bug

When for any reason fetching the version.json file fails, for example because the user lost their internet connection, Sentry ends up getting littered with TypeError: Load failed errors.

Screenshot 2023-03-27 at 11 10 53

Screenshot 2023-03-27 at 11 13 02

Wouldn't it make sense to wrap this call to fetch in a try/catch block?

Reproduction

Just a brand new skeleton project will do, with version.pollInterval set up (see #9089 (comment)). Create a build locally, run with npm run preview, and then kill the server. Or with a production site that uses this, turn off your WiFi.

Logs

No response

System Info

System:
    OS: macOS 13.2.1
    CPU: (10) arm64 Apple M1 Max
    Memory: 1.29 GB / 32.00 GB
    Shell: 3.6.0 - /opt/homebrew/bin/fish
  Binaries:
    Node: 16.17.0 - ~/Library/pnpm/node
    npm: 8.15.0 - ~/Library/pnpm/npm
  Browsers:
    Chrome: 111.0.5563.64
    Firefox: 111.0.1
    Safari: 16.3
  npmPackages:
    @sveltejs/adapter-node: ^1.2.3 => 1.2.3
    @sveltejs/kit: ^1.13.0 => 1.13.0
    svelte: ^3.57.0 => 3.57.0
    vite: ^4.2.1 => 4.2.1

Severity

annoyance

Additional Information

No response

@dummdidumm dummdidumm added bug Something isn't working ready to implement please submit PRs for these issues! labels Mar 27, 2023
@dummdidumm dummdidumm added this to the non-urgent milestone Mar 27, 2023
@elliott-with-the-longest-name-on-github
Copy link
Contributor

Okay, this is pretty funny. See the linked PR -- clearly we're handling the error and not re-throwing it, but the console is still littered with errors (at least in Chrome). We're handling the fetch failure, but it seems Chrome still reports the network error to the console. @kevinrenskers, does the fix in the PR still send errors to Sentry, or does Sentry (correctly, IMO) ignore the network failures because the fetch error is handled?

@kevinrenskers
Copy link
Author

It's normal that fetch errors are shown in the console when you catch them - but they shouldn't then make it into Sentry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ready to implement please submit PRs for these issues!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants
@kevinrenskers @dummdidumm @elliott-with-the-longest-name-on-github and others