Skip to content

⏳ Feat: Report errors to Sentry#776

Draft
shmeleva wants to merge 4 commits intomainfrom
feat/send-errors-to-sentry
Draft

⏳ Feat: Report errors to Sentry#776
shmeleva wants to merge 4 commits intomainfrom
feat/send-errors-to-sentry

Conversation

@shmeleva
Copy link
Contributor

@shmeleva shmeleva commented Apr 22, 2025

WHY:

Fixes #533

What has been changed (if possible, add screenshots, gifs, etc. )

  • I created two projects in Sentry: one for the backend (Node.js, BACKEND_SENTRY_DSN) and one for the frontend (SvelteKit, PUBLIC_FRONTEND_SENTRY_DSN).
  • All errors are captured in the middleware / hooks:
    • In Strapi, errors are handled by the 'global::error-capture' middleware.
    • In SvelteKit, errors are handled inside hooks.client.js and hooks.server.js.
  • I have not configured any filters or alerts in Sentry.

Testing:
There are some errors in Sentry :)

  • Strapi error capture can be tested by, for example, by trying to login to a candidate app with a wrong password.
  • I tested SvelteKit errors by simply throwing exceptions in both the server and client code.

TODO:

  • Move configuration to env vars.

Check off each of the following tasks as they are completed

  • I have reviewed the changes myself in this PR. Please check the self-review document
  • I have added or edited unit tests.
  • I have run the unit tests successfully.
  • I have run the e2e tests successfully.
  • I have tested this change on my own device.
  • I have tested this change on other devices (Using Browserstack is recommended).
  • I have tested my changes using the WAVE extension
  • I have tested my changes using keyboard navigation and screen-reading
  • I have added documentation where necessary.
  • Is there an existing issue linked to this PR?
  • I have cleaned up the commit history and checked the commits follow the guidelines

Copy link
Contributor

@kaljarv kaljarv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add settings to packages/app-shared/src/settings/staticSettings.ts for enabling Sentry (not enabled by default) and configuring the sourceMapsUploadOptions (for Sveltekit). We'll still be needing the envs for any secrets, tho.

It'd be great if also the imports from sentry as well as any Sentry.inits and plugin loads for Strapi and Sveltkit would be conditional on this setting.


// If you have a custom error handler, pass it to `handleErrorWithSentry`
export const handleError = handleErrorWithSentry();
// Note, since optionalHandleErrorWithSentry is assigned in a promise,
Copy link
Collaborator

@gogarufi gogarufi Jun 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a dealbreaker for conditional importing? :)

@@ -1,4 +1,5 @@
import * as Sentry from '@sentry/sveltekit';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that on the server side we benefit from conditional dynamic importing of Sentry.

@shmeleva
Copy link
Contributor Author

shmeleva commented Jun 1, 2025

Just created #778 in case if it's still needed.

@kaljarv kaljarv changed the title Feat: Report errors to Sentry ⏳ Feat: Report errors to Sentry Aug 23, 2025
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.

feat: runtime error logging

3 participants