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

Mark some BuildEnvironmentError exceptions as Warning and do not log them #4495

Merged
merged 7 commits into from Aug 22, 2018

Conversation

@humitos
Copy link
Member

@humitos humitos commented Aug 8, 2018

There are some exceptions risen while building documentation that are considered an ERROR from the Build perspective, but for our application are just a WARNING and shouldn't be logged as ERROR (sent to Sentry).

It's not an ERROR in our application but a WARNING that the user's documentation couldn't be built.

This will help us to find Sentry more useful than it's right now as we are getting close to have logged only things that need human/dev attention.

NOTE: this is the most important commit 1dd850a, the first one is a minimum refactor and the last one introduces some style changes

Closes: #4109

@humitos humitos requested a review from Aug 8, 2018
@humitos humitos force-pushed the humitos/build/log-exceptions branch from be3dcd6 to d6cc500 Aug 8, 2018
Copy link
Member

@ericholscher ericholscher left a comment

Looks good. Will definitely be nice not to have so much spam in sentry.

@ericholscher
Copy link
Member

@ericholscher ericholscher commented Aug 22, 2018

@humitos we just hit our limit again, we should fix the conflict here and ship it.

humitos added 7 commits Aug 22, 2018
There are some exceptions risen while building documentation that are
considered an ERROR from the Build perspective, but for our
application are just a WARNING and shouldn't be logged as ERROR (sent
to Sentry).

It's not an ERROR in our application but a WARNING that the user's
documentation couldn't be built.
Note: the changes were selected manually since there were many that
break the style that we want :(
@humitos humitos force-pushed the humitos/build/log-exceptions branch from 7511411 to 29f1f65 Aug 22, 2018
@humitos
Copy link
Member Author

@humitos humitos commented Aug 22, 2018

Rebased the branch. Once the tests passed, I will merge it.

@humitos humitos merged commit 883b383 into master Aug 22, 2018
1 check passed
@humitos humitos deleted the humitos/build/log-exceptions branch Aug 22, 2018
@stsewd
Copy link
Member

@stsewd stsewd commented Aug 22, 2018

This solves #3856? or we still need less logs?

@humitos
Copy link
Member Author

@humitos humitos commented Aug 22, 2018

I don't think it 100% solves it, but at least reduce the noise.

I'd like to get to the point to anything logged in Sentry requires manual/human attention. Currently, there are still lot of garbage there.

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.

3 participants