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.

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

LF-3914 Post (some) handled errors to Sentry #3025

Merged
merged 3 commits into from
Dec 5, 2023

Conversation

kathyavini
Copy link
Collaborator

@kathyavini kathyavini commented Dec 5, 2023

Update: we modified our Sentry permissions temporarily to allow troubleshooting from localhost. The issue format and stack trace look good and useable so I think it's fine to run on beta!

Description

Note: this relates to PR #3023 in that it piggy-backs on the Winston logger to get our errors to Sentry, but should be a standalone code change (i.e. doesn't depend on that PR's code changes).

This PR adds a custom Winston transport to send Error objects passed to Winston to Sentry, using Sentry.captureException(error), see Sentry docs). Because our needs were minimal, I wrote a basic transport, similar to this StackOverflow example.

I'm happy with the current output (although I wish it tracked users!). Just as a note, there is a more full-featured transport available here: winston-transport-sentry-node, (Source Code). I didn't use it because it has a bit more than we need (e.g. log levels), and I couldn't get it to work out-of-the-box, but it's a very popular package and always an option in the future.

Jira link: https://lite-farm.atlassian.net/browse/LF-3914

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain)

Tested by triggering the LF-3884 error from localhost with the environment code checks reversed (to run on development) and the beta sentry dsn key.

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have added "MISSING" for all new language tags to languages I don't speak
  • I have added the GNU General Public License to all new files

@kathyavini kathyavini added the enhancement New feature or request label Dec 5, 2023
@kathyavini kathyavini self-assigned this Dec 5, 2023
@kathyavini kathyavini requested review from a team as code owners December 5, 2023 19:48
@kathyavini kathyavini requested review from antsgar, SayakaOno and navDhammu and removed request for a team December 5, 2023 19:48
SayakaOno
SayakaOno previously approved these changes Dec 5, 2023
Copy link
Collaborator

@SayakaOno SayakaOno left a comment

Choose a reason for hiding this comment

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

Let's test it!
The Jira ticket is linked instead of "Sentry docs" in the description 😉

@kathyavini
Copy link
Collaborator Author

The Jira ticket is linked instead of "Sentry docs" in the description 😉

Ooops, thank you! 😁

@kathyavini kathyavini marked this pull request as draft December 5, 2023 21:25
@kathyavini kathyavini marked this pull request as ready for review December 5, 2023 21:40
@kathyavini kathyavini force-pushed the LF-3914-post-some-handled-errors-to-sentry branch 2 times, most recently from 83c146e to 6eba324 Compare December 5, 2023 22:17
@kathyavini kathyavini force-pushed the LF-3914-post-some-handled-errors-to-sentry branch from 6eba324 to 2b38e47 Compare December 5, 2023 22:18
@kathyavini kathyavini force-pushed the LF-3914-post-some-handled-errors-to-sentry branch from 2b38e47 to 0e9ec16 Compare December 5, 2023 22:26
@kathyavini kathyavini merged commit 09b2495 into integration Dec 5, 2023
2 checks passed
@kathyavini kathyavini deleted the LF-3914-post-some-handled-errors-to-sentry branch December 5, 2023 23:07
antsgar pushed a commit that referenced this pull request Dec 12, 2023
…rrors-to-sentry

LF-3914 Post (some) handled errors to Sentry
antsgar pushed a commit that referenced this pull request Dec 12, 2023
…rrors-to-sentry

LF-3914 Post (some) handled errors to Sentry
antsgar pushed a commit that referenced this pull request Dec 13, 2023
…rrors-to-sentry

LF-3914 Post (some) handled errors to Sentry
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants