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

Log request data when Travis errors #1631

Merged
merged 2 commits into from
Jul 20, 2018

Conversation

espadrine
Copy link
Member

@espadrine espadrine requested a review from paulmelnikow April 1, 2018 16:09
@shields-ci
Copy link

shields-ci commented Apr 1, 2018

Warnings
⚠️

This PR modified the server but none of the service tests. That's okay so long as it's refactoring existing code.

Messages
📖

✨ Thanks for your contribution to Shields, @espadrine!

Generated by 🚫 dangerJS

@paulmelnikow
Copy link
Member

Hi, there's an issue open for this #1611. I tend to think we should not bother to log these errors, as many of them will be spurious. For most services, we don't log the equivalent. Is there a reason to treat Travis differently?

@paulmelnikow paulmelnikow added service-badge New or updated service badge sentry Bugs found using Sentry labels Apr 2, 2018
@espadrine
Copy link
Member Author

It may be a transient error, or a real error. Either way, it was already logged, but it lacked information in the log for it to be useful: I can't tell what the problem actually was.

In general, I feel logging the request data and the vendor response is useful to investigate errors.

@paulmelnikow
Copy link
Member

I don't feel strongly about this.

@paulmelnikow paulmelnikow merged commit fd76819 into badges:master Jul 20, 2018
@shields-deployment
Copy link

This pull request was merged to master branch. Now this change is waiting for deployment.
Deploys usually happen every few weeks. After deployment changes are copied to gh-pages branch.

This badge displays deployment status:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sentry Bugs found using Sentry service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants