740773 - No error messages when sending report to a remote host fails

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[David Burns :automatedtester]

After working on bug 703789 it was noticed that Mozmill does not report error messages properly if Mozmill Dashboard blocks the data

Comment 1

[David Burns :automatedtester]

Attachment: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/12

Pointer to Github pull-request

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Clint, it would be great if this could be approved for Mozmill 1.5.10. Things like that are annoying when we have to investigate failures. Now with the validation function in place it's even unclear to the user if something fails.

Comment 3

[Jeff Hammel]

Comment on attachment 610860 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/12

wfm; the pull request is to master but we will probably want this on 2.0 and probably 1.5

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Given that no-one checked my comment on the pull request (which you should do before you give review here) I will copy it into the bug:

Wow, what a silly mistake. David, can you please make so that we only show the data['reason'] value here? The others we really don't need.

Also do we still show the host we send the report to? If not we should print the error reason in a new line.


So before we check this into older branches please get those comments addressed first.

Comment 5

[Dave Hunt [:davehunt] [he/him] ⌚BST]

I've made a suggestion on the Github pull request (repeated below, original here: https://github.com/mozautomation/mozmill/pull/12#issuecomment-4953820
), which addresses Henrik's comment:

If instead we use this:

    # Check if the report has been created
    if not 'ok' in data:
        raise Exception(data['reason'])

We get the failure as:

    Sending results to 'http://mozmill-crowd.blargon7.com/db/' failed (This document requires the field report_version).

Did you want me to put together a new pull request?

Comment 6

[cmtalbert]

(In reply to Dave Hunt (:davehunt) from comment #5)
> I've made a suggestion on the Github pull request (repeated below, original
> here: https://github.com/mozautomation/mozmill/pull/12#issuecomment-4953820
> ), which addresses Henrik's comment:
> 
> If instead we use this:
> 
>     # Check if the report has been created
>     if not 'ok' in data:
>         raise Exception(data['reason'])
> 
> We get the failure as:
> 
>     Sending results to 'http://mozmill-crowd.blargon7.com/db/' failed (This
> document requires the field report_version).
> 
> Did you want me to put together a new pull request?

Yes, let's do that. Can you make a new pull request.

Comment 7

[Dave Hunt [:davehunt] [he/him] ⌚BST]

New pull request is up: https://github.com/mozautomation/mozmill/pull/13

Comment 8

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/13

Pointer to Github pull-request

Comment 9

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/13#

Pointer to Github pull-request

Comment 10

[cmtalbert]

Comment on attachment 613745 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/13

looks great

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 610860 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/12

This attachment is not obsolete. It has been checked-in.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Landed with the merge on master:
https://github.com/mozautomation/mozmill/commit/a14344b369bd1fe7c6c8fc30c67290e7b2c5945a

Also I have cherry-picked both patches and landed those on hotfix-2.0:
https://github.com/mozautomation/mozmill/compare/61af56a...ae43517

Dave, can you please submit a combined patch for the hotfix-1.5 branch?

Comment 13

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Attachment: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/15

Pointer to Github pull-request

Comment 14

[Dave Hunt [:davehunt] [he/him] ⌚BST]

Comment on attachment 613926 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/15

Patch for hotfix-1.5 branch.

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Comment on attachment 613926 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/15

Looks good. Landed on hotfix-1.5:
https://github.com/mozautomation/mozmill/commit/b2f47fb26c203d842f35d89fe2ef6731608e3185

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

So we are done here. Thanks Dave!