-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Added TODO items #8458
base: main
Are you sure you want to change the base?
Added TODO items #8458
Conversation
TODOs/FIXMEs:
|
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.
PR Summary
This PR implements Sentry error tracking in the PromiseRejectionEffect component and simplifies the Avatar component by removing Recoil state management.
- Added Sentry.captureException in
/packages/twenty-front/src/modules/error-handler/components/PromiseRejectionEffect.tsx
for error tracking - Replaced Recoil's invalidAvatarUrlsState with local useState in
/packages/twenty-ui/src/display/avatar/components/Avatar.tsx
- Improved error message handling for ObjectMetadataItemNotFoundError with specific snackbar notifications
- Simplified Avatar component by making it more self-contained without external state dependencies
2 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile
@@ -11,7 +11,8 @@ export const PromiseRejectionEffect = () => { | |||
(event: PromiseRejectionEvent) => { | |||
const error = event.reason; | |||
|
|||
// TODO: connect Sentry here | |||
Sentry.captureException(error); | |||
|
|||
if (error instanceof ObjectMetadataItemNotFoundError) { | |||
enqueueSnackBar( | |||
`Error with custom object that cannot be found : ${event.reason}`, |
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.
style: Using event.reason here is redundant since we already have the error object. Consider using error.message for consistency with the else branch.
const error = event.reason; | ||
|
||
// TODO: connect Sentry here | ||
Sentry.captureException(error); |
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.
style: Consider wrapping Sentry.captureException in try/catch to prevent potential errors in error handling itself
Description
This PR adds Sentry and Removed Recoil
WIP