771425 - Uploading report sometimes fails with 'No JSON object could be decoded'

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

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

Sometimes we fail in uploading the report to the dashboard. The failure we are facing is the following:

INFO Passed: 201
INFO Failed: 0
INFO Skipped: 0
No JSON object could be decoded

We are failing somewhere in this code:
https://github.com/mozautomation/mozmill/blob/master/mozmill/mozmill/report.py#L97

Could even be inside an exception block where we are trying to decode the response and really don't output the right failure.

This happens for Mozmill 1.5 on and off but the code for Mozmill 2 is the same. So asking for blocking 2.0.

Comment 1

[cmtalbert]

I think this is an issue with the response from the server - we expect JSON back and it isn't.  We aren't throwing an exception so not sure what else we can do.  Going to mark as mozmill-next for now. Re nominate if we can reproduce it reliably.

Comment 2

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

This mostly correlates to our addons testrun. We should try to be able to reproduce it.

Comment 3

[sanketsaurav]

Hello! I want to work on this bug, and this is my first bug. Can anyone guide me where to start? Thank you.

Comment 4

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

Hi Sushant, I'm not fully sure if it was you who started on this bug already. Could you please let me know? Thanks.

Sanketsaurav, depending on the current status of this bug you might be able to take it or we have to find something else for you to work on. We have plenty of mentored bugs in our components. Thanks.

Comment 5

[Sushant Hiray [:sushant]]

Henrik,

Considering the bug has been out for so many days, I guess there might be some difficulties in recreating the bug.
If Sanketsaurav has managed to recreate the bug, then he can work on this :)
Else I will work given the fact that I've been able to reproduce the bug already.

Thanks,

Comment 6

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

Thank you Sushant! Sanketsaurav are you able to reproduce the problem? It might be that this can be always seen if you are behind an auth proxy, which you haven't authenticated against yet.

Comment 7

[sanketsaurav]

Henrik,

I've not reproduced the bug yet. This is my first bug, and would really like to get started with this. Please guide me.

(In reply to Henrik Skupin (:whimboo) from comment #6)
> Thank you Sushant! Sanketsaurav are you able to reproduce the problem? It
> might be that this can be always seen if you are behind an auth proxy, which
> you haven't authenticated against yet.

Comment 8

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

(In reply to sanketsaurav from comment #7)
> I've not reproduced the bug yet. This is my first bug, and would really like
> to get started with this. Please guide me.

Hm, I think that this problem is related to using a proxy, and that the response we get back is not JSON data but a simple string. I think that you should be able to reproduce it when you setup a local proxy server on your computer as best with authentication. When Mozmill then tries to send the report and no credentials have been given, you will most likely see a string as return value of the urllib method. Decoding it should fail. If you can't do that simply tweak the code and overwrite the return value with various values (string, number...) and check how the decode method reacts on it.

Comment 9

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

Frederic would like to work on this bug. So assigning him now.

Comment 10

[Fredrik Broman [:bromaniac]]

Attachment: 0001-Bug-771425-Uploading-report-sometimes-fails-with-No-.patch

Yes this is related to being behind a proxy you haven't authenticated to. The proxy then returns HTTP error 407 and some html but report.py doesn't care about the HTTP error, it just tries to parse the html as json. This of course fails.

I've added a new try in front of the json parser and if the parser fails the new except prints the HTTP error code and string.

I made some simple tests with a local squid3 proxy and couchdb:

Without being authenticated:
Sending results to 'http://fredrikbroman.se/db' failed (407 - Proxy Authentication Required).

Without proxy but sending to a web server not couchdb:
Sending results to 'http://fredrikbroman.se/db' failed (404 - Not Found).

Without proxy but to a couchdb but non-existant db:
Sending results to 'http://localhost:5984/fb' failed (no_db_file).

Without proxy to a couchdb and existing db:
Report document created at 'http://localhost:5984/db12af5e09abb24a556a1853388a000681'

So it looks like the patch is working!

Comment 11

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

Wow that looks fantastic! Thanks a lot for working on it Fredrik. And everything without any help. I'm impressed! Given that your examples above work, I would even consider adding it to the next Mozmill 2.0.3 release. I will have a look at the patch in a bit.

Comment 12

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

Comment on attachment 8349563 [details] [diff] [review]
0001-Bug-771425-Uploading-report-sometimes-fails-with-No-.patch

Review of attachment 8349563 [details] [diff] [review]:
-----------------------------------------------------------------

It looks great. I only have a nit which needs an update. See my inline comment. With that fixed you have my r+! Thanks a lot!

::: mozmill/mozmill/report.py
@@ +112,5 @@
> +                                                              data['reason'])
> +            except ValueError:
> +              print "Sending results to '%s' failed (%s - %s)." % (report_url,
> +                                                            str(e.code),
> +                                                            e.reason)

I would simply use `str(e)` here and not split it up. That way we also know the type of exception thrown.

Comment 13

[Fredrik Broman [:bromaniac]]

Attachment: 0001-Bug-771425-Uploading-report-sometimes-fails-with-No-.patch

Thank you for your kind words Henrik! Fixing the bug wasn't that hard but setting up the environment with proxy etc to be able to reproduce it and test the code took some time.

Anyways, here is the new patch. I agree it looks better:
Sending results to 'http://localhost:5984/db' failed (HTTP Error 407: Proxy Authentication Required).

Comment 14

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

Comment on attachment 8350147 [details] [diff] [review]
0001-Bug-771425-Uploading-report-sometimes-fails-with-No-.patch

Review of attachment 8350147 [details] [diff] [review]:
-----------------------------------------------------------------

I think that looks perfect now. I'm not a fan of those nested try/except blocks but any kind of refactoring we can do later. I will get this landed and fold it into our upcoming Mozmill 2.0.3 release. Good to see that this nasty bug has been fixed.

Comment 15

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

Landed as:
https://github.com/mozilla/mozmill/commit/7eded93716d620f1f3c8473e9825e21718a92b56 (master)
https://github.com/mozilla/mozmill/commit/0945f3835e29bb7c07a4a33885953da495a59fe7 (hotfix-2.0)

Fredrik, please let me know what else you would like to work on or any special interests in what you want to learn. We should be able to find the right next bug for you.