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
[CORL-1293] Sentry Error Integration #3109
Conversation
502a281
to
7e056cb
Compare
// Parse the config from the element text, it should always be a valid JSON | ||
// string if the element is available. We should allow the error to bubble if | ||
// we are unable to parse it. | ||
const config: StaticConfig = JSON.parse(element.innerText); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a bit scary, is there a reason we can't pass these values around like other config values?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want the error reporter setup as early as possible. Otherwise we won't capture network related errors. This is the same mechanism that we pass updates to the public path (See src/core/build/publicPath.js
).
return { | ||
id: user.id, | ||
username: user.username || undefined, | ||
email: user.email || undefined, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to send email to the error reporter? I'd feel more comfortable if it was just id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The email was to aid in responding to users that experience errors, but we can certainly remove it!
What does this PR do?
apollo-server-express
with vanilla implementation.What changes to the GraphQL/Database Schema does this PR introduce?
None.
How do I test this PR?
TBD