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.

Sign up for GitHub

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

Send error details to Sentry #1166

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Send error details to Sentry #1166

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
edb371c
Send error details to Sentry
daniellemaxwell Mar 18, 2024
721fa16
Merge branch 'main' into sc-24411
bbengfort Mar 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion web/gds-user-ui/src/modules/auth/login/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const StartPage: React.FC = () => {
}

// catch this error in sentry
Sentry.captureException(err);
Sentry.captureException(err?.original);
Copy link
Collaborator Author

@daniellemaxwell daniellemaxwell Mar 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

After reading this comment, it seems that Sentry didn't think err is an error object: getsentry/sentry-react-native#391 (comment)

err?.original returns an object inside the error object with more details, so this could potentially resolve the issue.

I should note that if a user enters incorrect log in details, they see an error message explaining that the username or password is invalid. This change will not affect that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

When it comes to the ?. syntax, what will be passed into captureException if the original property is not on err? Probably we would just want to pass in err in that case, right?

I may just be unfamiliar with ?. but how does this code differ?

try {
    Sentry.captureException(err);
} catch {
     Sentry.captureException(err?.original);
}

or is it more like:

if (err?.original) {
    Sentry.captureException(err?.original);
} else {
    Sentry.captureException(err);
}

Or is it that we can always guarantee that there is an err.original on the error object?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

err returns an object with the error, error code, error description, and another object (original) that contains the error and error description.
Screenshot 2024-03-19 at 2 50 25 PM

?. will check if err is undefined before attempting to read what's inside the original object. It was added to prevent an undefined error from being returned.

If original isn't included, then err wouldn't be passed into the exception. I didn't include an else because we currently pass err into Sentry.captureException and get an error message. I thought possibly passing in this object directly could be helpful. The error didn't trigger in development mode for me to confirm if this is truly the solution.

From what I can see, err.original will be included in the err object. However, this is coming from Auth0 so I can't quite guarantee this won't ever change.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I see now that this is already in a try/catch block for Auth0 specific handling. Thank you for clarifying this!

}
};
return (
Expand Down
Loading