In bug 1499246 we have been chasing down issues with downloading binaries from tooltool at Bitbar. I finally got a pointer to where the problem might be occurring once I made a change to log exceptions in fetch_file. I think this would be a good change to make in general to help in diagnosing issues. I'll attach my wip/demo patch for your perusal.

I found this very useful. Let's make it official.

Comment on attachment 9020349 [details] [diff] [review] tooltool-fetch-logging.patch Review of attachment 9020349 [details] [diff] [review]: ----------------------------------------------------------------- would using log.info(..., exc_info=True) be a better way of logging the exceptions?

I can try it out and see how it does. Give me a bit and I'll report back.

I wonder though if that will introduce oranges due to the exception stack in the logs?

I ran a full test run on try <https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&tier=1%2C2%2C3&revision=f13f8079d4c4af6e0a352be2dde454cf09d78dab> with an additional patch <https://hg.mozilla.org/try/rev/80ebf7f1cdf55a9b867e6e7166a0078c58bb8afe> I didn't see this failure once so it is not so common as to turn a things perma orange right away. I'm not sure we want to change the behavior to turn these into oranges though, but I could see how that could be a good thing. I'll let you make that call. Let me know and if you like, I'll fold it into the original patch and submit that for review.

I did see this while testing the android hardware which is where I originally found this issue: <https://treeherder.mozilla.org/logviewer.html#?job_id=208148925&repo=try&lineNumber=1836-1853>

folded exc_info=True into the patch.

Comment on attachment 9022033 [details] [diff] [review] tooltool-fetch-logging.patch :bc sorry for late review, it was a busy week can you also apply the same patch also to https://github.com/mozilla/build-tooltool/blob/master/tooltool.py

https://github.com/mozilla/build-tooltool/pull/45 with a change to remove the extraneous e variable and after some git foobars.

Pushed by bclary@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/4034fbcc0a55 Update tooltool.py fetch_file to log.info exceptions instead of log.debug, r=garbas

Note I removed the extraneous e from the version I pushed as well.

Backout by csabou@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a725d6063ad0 Backed out changeset 4034fbcc0a55 for causing mass mochitest failures.

This was due to the rebuilding of the docker images and bug 1503756 I believe. I should have caught it if I had done a full set of unit tests on try when the image was rebuilt.

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/565215cf2e5e Update tooltool.py fetch_file to log.info exceptions instead of log.debug, r=garbas

Backout by csabou@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/48720735b142 Backed out changeset 565215cf2e5e for causing mass mda failures. CLOSED TREE

Bug 1503756 was pushed here https://bugzilla.mozilla.org/show_bug.cgi?id=1503756#c31 and eventually got backed out. After that backout the changes pushed here went on creating new mass failures on mda test. Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=211901297&repo=autoland&lineNumber=5167 Backout push for 1503756: https://treeherder.mozilla.org/#/jobs?repo=autoland&searchStr=mda&group_state=expanded&selectedJob=211879519&revision=2a9517f92a0d29bd8280e7f6fd897f19c4cf68c3

I didn't land this. I think gps landed it as part of the attempt to fix the docker image building. If it is landed again as part of the docker image fix, and the docker image fix is backed out this should be backed out at the same time as it generates a new docker image which will also fail.

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/9a627c744233 Update tooltool.py fetch_file to log.info exceptions instead of log.debug, r=garbas