1089805 - BzAPI compat layer generates HTTP 404 errors for cases where legacy BzAPI used HTTP 400

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Extensions, defect)

Description

[Ed Morley [:emorley]]

In bug 1062305, mcMerge is intermittently failing to update bugs. This has been tracked to cases where the assignee we are trying to set, does not exist in Bugzilla (eg someone uses a different hg commit email to their Bugzilla email).

mcMerge has handled this case fine for some time (if the first request returned a HTTP 400 it would try again but without the assignee change [1]), however after switching from the original BzAPI to the compatibility layer, these bad requests no longer return HTTP 400.

eg:

--

Request URL: 	https://bugzilla.mozilla.org/bzapi/bug/1062305?<snip username and password>
Status Code: 	HTTP/1.1 200 OK

Request Body:
{"comments":[{"creation_time":"2014-10-27T19:43:12.769Z","creator":{"email":"emorley@<snip>.com"},"is_private":0,"text":"Testing"}],"id":"1062305","whiteboard":"","assigned_to":{"name":"trev.saunders@<snip>.com"},"last_change_time":"2014-10-27T19:28:45Z","update_token":"<snip>"}

Response Body:
{"documentation":"https://wiki.mozilla.org/Bugzilla:BzAPI","error":true,"code":51,"message":"There is no user named 'trev.saunders@<snip>.com'. Either you mis-typed the name or that user has not yet registered for a Bugzilla account."}

--

Was this change intended?

[1] http://mxr.mozilla.org/webtools-central/source/tbpl/mcmerge/js/Step.js#242

Comment 1

[David Lawrence [:dkl]]

It is returning the same status codes as the native upstream API as we do not override them for the BzAPI compat extension. Currently the assignee (or more generically, object_does_not_exist) error returns 404 instead of 400 which the BzAPI proxy used to return.

I can do one of 3 things
 
1) File a bug upstream to change the current native behavior to return 400 instead of 404 for object_does_not_exist type errors. If we do that then the BzAPI extension will automatically return the same.
2) Add a hook to allow the status codes to be overridden by extensions so that BzAPI can change it to 400 instead to remain backwards compatible.
3) Or leave it as-is and push for mcmerge to use the new error code instead of 400.

Thoughts? glob?

dkl

Comment 2

[Ed Morley [:emorley]]

(In reply to David Lawrence [:dkl] from comment #1)
> It is returning the same status codes as the native upstream API as we do
> not override them for the BzAPI compat extension. Currently the assignee (or
> more generically, object_does_not_exist) error returns 404 instead of 400
> which the BzAPI proxy used to return.

This isn't what I'm seeing - we're getting 200s not 404s.

> 3) Or leave it as-is and push for mcmerge to use the new error code instead
> of 400.

I'm happy to change mcMerge to look for 404s (rather than 400s), once bzapi compat layer returns them instead of 200s.

Comment 3

[:glob ✱]

(In reply to David Lawrence [:dkl] from comment #1)
> 1) File a bug upstream to change the current native behavior to return 400
> instead of 404 for object_does_not_exist type errors. If we do that then the
> BzAPI extension will automatically return the same.
> 2) Add a hook to allow the status codes to be overridden by extensions so
> that BzAPI can change it to 400 instead to remain backwards compatible.
> 3) Or leave it as-is and push for mcmerge to use the new error code instead
> of 400.
> 
> Thoughts? glob?

#2 gets my vote.

Comment 4

[David Lawrence [:dkl]]

Attachment: 1089805_1.patch

Basically backport of bug 1094858 (r+ waiting on approval) with the simple addition of the status code change in the BzAPI extension.

dkl

Comment 5

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8518993 [details] [diff] [review]
1089805_1.patch

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

r=dylan

Comment 6

[:glob ✱]

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   c2533c1..2d223dc  master -> master

Comment 7

[Ed Morley [:emorley]]

This unfortunately hasn't worked.

Request URL: 	https://bugzilla.mozilla.org/bzapi/bug/1069823?username=emorley%40mozilla.com&password=<snip>
Request Method: 	PUT
Status Code: 	HTTP/1.1 200 OK

Request Body
{"comments":[{"creation_time":"2014-11-12T01:27:45.561Z","creator":{"email":"[e-mail]"},"is_private":0,"text":"Testing for bug 1062305."}],"id":"1069823","whiteboard":"","assigned_to":{"name":"[e-mail]"},"last_change_time":"2014-10-15T08:09:43Z","update_token":"<snip>"}

Response Body Δ0ms
{"documentation":"https://wiki.mozilla.org/Bugzilla:BzAPI","error":true,"code":51,"message":"There is no user named 'eflores@<snip>.com'. Either you mis-typed the name or that user has not yet registered for a Bugzilla account."}


-> Expected status code != 200.

Comment 8

[David Lawrence [:dkl]]

(In reply to Ed Morley [:edmorley] from comment #7)
> This unfortunately hasn't worked.
> 
> Request URL: 
> https://bugzilla.mozilla.org/bzapi/bug/1069823?username=emorley%40mozilla.
> com&password=<snip>
> Request Method: 	PUT
> Status Code: 	HTTP/1.1 200 OK
> 
> Request Body
> {"comments":[{"creation_time":"2014-11-12T01:27:45.561Z","creator":{"email":
> "[e-mail]"},"is_private":0,"text":"Testing for bug
> 1062305."}],"id":"1069823","whiteboard":"","assigned_to":{"name":
> "[e-mail]"},"last_change_time":"2014-10-15T08:09:43Z",
> "update_token":"<snip>"}
> 
> Response Body Δ0ms
> {"documentation":"https://wiki.mozilla.org/Bugzilla:BzAPI","error":true,
> "code":51,"message":"There is no user named 'eflores@<snip>.com'. Either you
> mis-typed the name or that user has not yet registered for a Bugzilla
> account."}
> 
> 
> -> Expected status code != 200.

This is very strange. I have tested this locally, on bugzilla.allizom.org (stage) and now on production and I still get 400 BAD REQUEST in all cases. I get the same exact error message as you get as well.

Could there be a proxy between your system and the production bugzilla that is altering the status code or possibly a bug in the client library your code is using?

glob, dylan, thoughts?

dkl

Comment 9

[Ed Morley [:emorley]]

That exact request and response is what is recorded by Firefox dev tools console, after the request is made (everything is client side). I don't know how a bug in any library could make a difference here, since if we were to recreate that exact same request using Curl, we should see the same response surely?

Comment 10

[Ed Morley [:emorley]]

Attachment: Curl POC

To use this script, export bugzilla USERNAME and PASSWORD prior to running.

It seems the difference between correct (HTTP 400) and incorrect (HTTP 200) behaviour is passing "--compressed" to Curl, or more specifically, adding the "Accept-Encoding: gzip" header.

Is this an issue with Zeus gzipping clobbering the HTTP result?

Comment 11

[Ed Morley [:emorley]]

Attachment: Curl POC

(Sorry fixed typo)

Comment 12

[Ed Morley [:emorley]]

$ ./bug1089805-poc.sh
Using --compressed:

HTTP/1.1 200 OK
Date: Wed, 12 Nov 2014 18:15:07 GMT
Server: Apache
Etag: ML6HmwR3C4e2pCgi5A8lLw
Access-control-allow-headers: origin, content-type, accept
X-xss-protection: 1; mode=block
Access-control-allow-origin: *
Strict-transport-security: max-age=31536000; includeSubDomains
X-frame-options: SAMEORIGIN
X-content-type-options: nosniff
Set-Cookie: Bugzilla_login=~SNIP~; path=/; secure; HttpOnly
Set-Cookie: Bugzilla_logincookie=~SNIP~; path=/; secure; HttpOnly
X-Backend-Server: web2.bugs.scl3.mozilla.com
Vary: Accept-Encoding
Content-Encoding: gzip
Content-Length: 189
Content-Type: application/json; charset=UTF-8

{"documentation":"https://wiki.mozilla.org/Bugzilla:BzAPI","error":true,"code":51,"message":"There is no user named 'eflores@~SNIP~.com'. Either you mis-typed the name or that user has not yet registered for a Bugzilla account."}

Without --compressed:

HTTP/1.1 400 Bad Request
Server: Apache
X-Backend-Server: web4.bugs.scl3.mozilla.com
Vary: Accept-Encoding
Content-Type: application/json; charset=UTF-8
Strict-transport-security: max-age=31536000; includeSubDomains
Date: Wed, 12 Nov 2014 18:15:08 GMT
X-xss-protection: 1; mode=block
Transfer-Encoding: chunked
Access-control-allow-origin: *
Etag: ML6HmwR3C4e2pCgi5A8lLw
X-content-type-options: nosniff
Connection: close
Set-Cookie: Bugzilla_login=~SNIP~; path=/; secure; HttpOnly
Set-Cookie: Bugzilla_logincookie=~SNIP~; path=/; secure; HttpOnly
X-frame-options: SAMEORIGIN
Access-control-allow-headers: origin, content-type, accept

{"documentation":"https://wiki.mozilla.org/Bugzilla:BzAPI","error":true,"code":51,"message":"There is no user named 'eflores@~SNIP~.com'. Either you mis-typed the name or that user has not yet registered for a Bugzilla account."}

Comment 13

[David Lawrence [:dkl]]

Ok I have been able to reproduce now using curl and I see the same behavior as well :) I do not see the same behavior locally so it must be Zeus related (I assume) and not sure what to do going forward.

Moving over to Infrastructure for further investigation.

fubar, is there anything configuration wise on the proxy that could be causing the status code to be overridden and can it be set to pass the proper code through when it sees generates gzipped content?

Thanks
dkl

Comment 14

[Kendall Libby [:fubar] (he/him)]

I'm getting the same discrepancy when going directly to a web head, so it's not the load balancers.

Comment 15

[Ed Morley [:emorley]]

This issue (comment 12) also explains:

(In reply to Ed Morley [:edmorley] from comment #2)
> (In reply to David Lawrence [:dkl] from comment #1)
> > It is returning the same status codes as the native upstream API as we do
> > not override them for the BzAPI compat extension. Currently the assignee (or
> > more generically, object_does_not_exist) error returns 404 instead of 400
> > which the BzAPI proxy used to return.
> 
> This isn't what I'm seeing - we're getting 200s not 404s.

Comment 16

[David Lawrence [:dkl]]

(In reply to Kendall Libby [:fubar] from comment #14)
> I'm getting the same discrepancy when going directly to a web head, so it's
> not the load balancers.

Could I see what the current apache configuration is from the webhead to see if I can reproduce locally?
dkl

Comment 17

[Ed Morley [:emorley]]

Simpler POC:

Good:
$ curl -s -i -X PUT https://bugzilla.mozilla.org/bzapi/bug/1069823 | head -n 1
HTTP/1.1 401 Authorization Required

Bad:
$ curl -s -i -X PUT -H 'Accept-Encoding: gzip' https://bugzilla.mozilla.org/bzapi/bug/1069823 | head -n 1
HTTP/1.1 200 OK

Comment 18

[Ed Morley [:emorley]]

Affects native REST too:

$ curl -s -i -X PUT https://bugzilla.mozilla.org/rest/bug/1069823 | head -n 1
HTTP/1.1 401 Authorization Required

$ curl -s -i -X PUT -H 'Accept-Encoding: gzip' https://bugzilla.mozilla.org/rest/bug/1069823 | head -n 1
HTTP/1.1 200 OK

Comment 19

[:glob ✱]

this looks like bug 1036802

Comment 20

[Ed Morley [:emorley]]

(In reply to Byron Jones ‹:glob› from comment #19)
> this looks like bug 1036802

Doh, seeing that would have saved so much time :-/

Morphing this bug to be about what actually landed here, and bug 1036802 can be about the bmo-infra specific issue when using gzip encoding (I'll move the needinfo over there).

On a separate but related note, is it intended that we have two hooks that do very similar things?
webservice_error_codes: https://github.com/mozilla/webtools-bmo-bugzilla/blob/2d223dcbe7bf5c045a11ea489a0964129518731e/extensions/BzAPI/Extension.pm#L105
webservice_status_code_map: https://github.com/mozilla/webtools-bmo-bugzilla/blob/2d223dcbe7bf5c045a11ea489a0964129518731e/extensions/BzAPI/Extension.pm#L173