Closed Bug 1360498 Opened 2 years ago Closed 2 years ago

Add update snippet URL to failure message in case of network issue

Categories

(Testing :: Marionette, defect)

Version 3
defect
Not set

Tracking

(firefox55 fixed, firefox56 fixed)

RESOLVED FIXED
mozilla56
Tracking Status
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: intermittent-bug-filer, Assigned: raajitr, Mentored)

Details

(Keywords: good-first-bug, intermittent-failure, pi-marionette-firefox-puppeteer, Whiteboard: [lang=py])

Attachments

(1 file)

This is happening when we are trying to fetch the update snippet from the AUS server. Not sure from the log why it happens. Maybe a networking issue at this time.

It might be helpful to also add the requested URL to the exception message.

https://dxr.mozilla.org/mozilla-central/rev/2cca333f546f38860f84940d4c72d7470a3410f4/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py#358


To get started on this bug please read:

https://wiki.mozilla.org/User:Mjzffr/New_Contributors
https://developer.mozilla.org/en-US/docs/Mozilla/QA/Firefox_UI_tests
Mentor: hskupin
Keywords: good-first-bug
Whiteboard: [lang=py]
Attachment #8865095 - Flags: review?(hskupin)
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review140274

::: commit-message-0c26d:1
(Diff revision 1)
> +Bug 1360498 - Added requested URL to the exception message

Can you please update the commit message so it says what exactly has been added? It should be clear from reading the hg changelog what has been done here.

::: testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py:369
(Diff revision 1)
>          try:
>              response = urllib2.urlopen(update_url)
>              return response.read()
>          except urllib2.URLError:
>              exc, val, tb = sys.exc_info()
> -            raise Exception, "Failed to retrieve update snippet: {}".format(val), tb
> +            raise Exception, "Failed to retrieve update snippet ({}): {}".format(update_url, val), tb

Maybe we better word it like:

"Failed to retrieve '{}': {}".
Attachment #8865095 - Flags: review?(hskupin) → review-
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review140274

> Can you please update the commit message so it says what exactly has been added? It should be clear from reading the hg changelog what has been done here.

> Bug 1360498 - Single quotes for URL in exception message of 'get_update_snippet' method

This is not what we should have in the hg log for this commit. So I will give you an example how it should look like, which also makes it easy to parse when going through the history.

> Bug 1360498 - Add update snippet URL to failure message if network issues occur.
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review141002

Please update the commit message as suggested. Then this patch is good to get landed. Thanks.
Attachment #8865095 - Flags: review?(hskupin) → review-
Assignee: nobody → raajit.raj
Status: NEW → ASSIGNED
Its wierd, I also got scolded for commit messages from my boss today.
I hope it's fine now.

Thanks
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review141530

::: testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py:369
(Diff revisions 2 - 3)
>          try:
>              response = urllib2.urlopen(update_url)
>              return response.read()
>          except urllib2.URLError:
>              exc, val, tb = sys.exc_info()
> -            raise Exception, "Failed to retrieve update snippet '{}': {}".format(update_url, val), tb
> +            raise Exception, "Failed to retrieve update snippet '{}': {}".format(val), tb

Why did you remove the first format parameter?
Attachment #8865095 - Flags: review?(hskupin) → review-
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review142004

Looks fine now. For the future please try to respond to open issues when you fixed them or have questions.
Attachment #8865095 - Flags: review?(hskupin) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s c2938b04051b -d ada4d29af826: rebasing 395461:c2938b04051b "Bug 1360498 - Added update snippet URL to failure in case of network issue r=whimboo" (tip)
merging testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py
warning: conflicts while merging testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
It looks like your changes are based on an older version of the repository. Please make sure to rebase against central, and push again to mozreview. Thanks.
Flags: needinfo?(raajit.raj)
I have updated my repository and pushed it again.

Thanks,
Flags: needinfo?(raajit.raj)
I will have a look tomorrow. Sorry that I missed the last comment.
Flags: needinfo?(hskupin)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2112c2dd7175
Added update snippet URL to failure in case of network issue r=whimboo
Component: Firefox UI Tests → Marionette
Flags: needinfo?(hskupin)
QA Contact: hskupin
Summary: Intermittent test_fallback_update.py TestFallbackUpdate.test_update | URLError: <urlopen error Failed to retrieve update snippet: [Errno 11001] getaddrinfo failed> → Add update snippet URL to failure message in case of network issue
Sorry, this had to be backed out for line 369 in testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py being too long. r=backout

https://hg.mozilla.org/integration/autoland/rev/ccce11a6183bbac03753e7c26236d8e7cbf11e5a

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2112c2dd7175088f3ef3a102ff944ac1a32ef584&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=100429575&repo=autoland
> TEST-UNEXPECTED-ERROR | /home/worker/checkouts/gecko/testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py:369:100 | line too long (101 > 99 characters) (E501)

Please fix the issue and submit an updated patch.
Flags: needinfo?(raajit.raj)
I have fixed it at my local end but getting an error when I push.

abort: Review request is submitted or discarded.
You must reopen the review request before it can be updated.
Review requests should only be reopened if your changes have not landed or have
been backed out - file new bugs for follow-up work.

Do I have to file a new bug?
Flags: needinfo?(raajit.raj)
I think I got it.

Have resubmitted with an updated patch.
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review146924

::: testing/marionette/puppeteer/firefox/firefox_puppeteer/api/software_update.py:370
(Diff revisions 5 - 6)
>              response = urllib2.urlopen(update_url)
>              return response.read()
>          except urllib2.URLError:
>              exc, val, tb = sys.exc_info()
> -            raise Exception, "Failed to retrieve update snippet '{}': {}".format(update_url, val), tb
> +            raise Exception, "Failed to retrieve update snippet '{}': {}".format(update_url,
> +                                                                                 val), tb

It would be better when you move all parameters to the next line with an indentation of 4 blanks.
Attachment #8865095 - Flags: review+ → review?(hskupin)
Raajit, can you please submit an update for the patch? Thanks.
Flags: needinfo?(raajit.raj)
Hi whimboo,

Sorry was away for few days. 
I have updated the patch please let me know if any changes is required.
Flags: needinfo?(raajit.raj)
Comment on attachment 8865095 [details]
Bug 1360498 - Added update snippet URL to failure in case of network issue

https://reviewboard.mozilla.org/r/136768/#review153004

Sorry that I missed the last update. This looks fine now. Thanks!
Attachment #8865095 - Flags: review?(hskupin) → review+
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 1 changes to 1 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev d560844e2758 needs "Bug N" or "No bug" in the commit message.
remote: Raajit Raj <raajit.raj@gmail.com>
remote: Bug - 1360498 Added update snippet URL to failure in case of network issue r=whimboo
remote: 
remote: MozReview-Commit-ID: GW0eCvg65WP
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote
Raajit, you modified the commit message again, so that it is invalid. Can you please fix it?
Flags: needinfo?(raajit.raj)
It would be great if we can get this finally fixed. Raajit, do you think you can actually do the remaining change?
Hi. If this is not yet fixed . I would like to work on this .
Flags: needinfo?(raajit.raj)
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1b802e268e97
Added update snippet URL to failure in case of network issue r=whimboo
https://hg.mozilla.org/mozilla-central/rev/1b802e268e97
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.