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

catch transcript errors, report block ID with error #42

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

ashultz0
Copy link
Contributor

@ashultz0 ashultz0 commented Jul 25, 2023

found by inspecting newrelic logs - we are in fact already suppressing errors, but they are boring ones we would prefer not to even log about and we'd like a bit more info when we suppress stuff

locally tested to make sure it still works and by forcing a fake error to see the improved log

Merge checklist:
Check off if complete or not applicable:

  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Fixup commits are squashed away
  • Unit tests added/updated
  • Manual testing instructions provided
  • Noted any: Concerns, dependencies, migration issues, deadlines, tickets

…fully

also tell us which block has an error which otherwise requires log spelunking

the general exception catch should be only for surprises, we know this
happens so let's handle it before it gets that far
Copy link
Member

@schenedx schenedx left a comment

Choose a reason for hiding this comment

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

👍 LGTM

Copy link
Member

@rijuma rijuma left a comment

Choose a reason for hiding this comment

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

LGTM

@ashultz0 ashultz0 merged commit fe17a46 into main Jul 25, 2023
4 checks passed
@ashultz0 ashultz0 deleted the ashultz0/better-error branch July 25, 2023 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants