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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't depend on specific data when catching exception #5326

Merged
merged 2 commits into from Feb 21, 2019

Conversation

@stsewd
Copy link
Member

@stsewd stsewd commented Feb 20, 2019

We are catching all exceptions here,
but we are depending on self.project and self.version
be correctly returned from the api.
This doesn't always happen, and it's raising another error.

We should catch more specific errors, but we are not seeing those errors bc of this error (hope that makes sense 馃憖 )

Sentry issue https://sentry.io/organizations/read-the-docs/issues/754905187/?project=148442&referrer=RegressionActivityEmail&statsPeriod=14d

We are catching all exceptions here,
but we are depending on self.project and self.version
be correctly returned from the api.
This doesn't always happens, and it's raising another error.
@stsewd stsewd requested a review from Feb 20, 2019
@stsewd stsewd changed the title Don't depend or specific data Don't depend on specific data when catching exception Feb 20, 2019
Copy link
Member

@humitos humitos left a comment

Make sense.

I think we have other places where this also happens, too. We can take care of them in the next iteration.

readthedocs/projects/tasks.py Show resolved Hide resolved
@ericholscher ericholscher merged commit ed8dd29 into readthedocs:master Feb 21, 2019
1 check passed
@stsewd stsewd deleted the protect-against-api-failure branch Feb 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants