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

Fix frequent LookupError exceptions #282

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

currycoder
Copy link
Contributor

This change attempts to fix the LookupError exceptions which we have been seeing frequently on sentry for production lite-hmrc. Unfortunately, this particular exception is very tricky to reproduce - so it's difficult to know if this will fix it or not. This PR takes a bit of an educated guess that the LookupErrors started when we upgraded sentry-sdk to 2.X; looking at the errors it seems there are issues accessing the ContextVar current_scope and it seems from looking at the sentry-sdk codebase that there has been work around this between 1.X and 2.X.

We don't have the luxury of time to look too deeply in to this - as it's not a blocking error and just an annoyance.

Comment on lines -81 to -82
ports:
- 6379:6379
Copy link
Contributor

Choose a reason for hiding this comment

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

can I ask what this change does?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually nvm I just read your commit messages which answered my question

@@ -338,7 +338,7 @@ def sort_dtos_by_date(input_dtos):

def log_to_sentry(message, extra=None, level="info"):
extra = extra or {}
with sentry_sdk.new_scope() as scope:
with sentry_sdk.push_scope() as scope:
Copy link
Contributor

Choose a reason for hiding this comment

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

I see that this is what we had before (https://github.com/uktrade/lite-hmrc/pull/274/files) so am I right to infer that the main change for this PR that we think will solve the issue is the version downgrade to 1.x ?

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