Closed joshgachnang closed 6 months ago
PR Description updated to latest commit (https://github.com/FlourishHealth/ferns-api/commit/ad252957fdbaef30eccdd0ad6a358bca41e12cce)
⏱️ Estimated effort to review [1-5] | 2, because the changes are straightforward and localized to a specific functionality (error handling for Sentry message capturing). The added try-catch block and error logging are simple to understand and review. |
🧪 Relevant tests | No |
🔍 Possible issues | Error Context: The error logging within the catch block logs the error message but does not provide context about where the error occurred or under what circumstances. It might be beneficial to include more information about the error context to aid in debugging. |
🔒 Security concerns | No |
Category | Suggestions |
Best practice |
Check if Sentry is initialized before capturing messages.___ **Consider checking if Sentry is initialized before attempting to capture a message. Thiscan prevent unnecessary try-catch blocks and ensure that Sentry's state is correctly managed before attempting to log messages. This approach is cleaner and more in line with Sentry's recommended usage patterns.** [src/api.ts [574-577]](https://github.com/FlourishHealth/ferns-api/pull/377/files#diff-769911c416ccf8514d8fd941ae0abe8fb5c606ade0c218e22151a5f5f9f3d700R574-R577) ```diff -try { +if (Sentry.isInitialized()) { Sentry.captureMessage(msg); -} catch (error) { - logger.error(`Error capturing message: ${error}`); +} else { + logger.warn('Sentry is not initialized.'); } ``` |
User description
TS suggested isInitialized was a real function, but that doesn't appear to be true.
Type
bug_fix
Description
Changes walkthrough
api.ts
Improved Error Handling for Sentry Message Capturing
src/api.ts
Sentry.captureMessage
in a try-catch block to handle potentialerrors.
with Sentry.