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

RESOLVED FIXED

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: emorley, Assigned: dkl)

Tracking

Production
Dependency tree / graph

Details

Attachments

(2 attachments, 1 obsolete attachment)

5.12 KB, patch
dylan
: review+
Details | Diff | Splinter Review
886 bytes, application/x-shellscript
Details
(Reporter)

Description

4 years ago
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
(Assignee)

Comment 1

4 years ago
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
Assignee: nobody → dkl
Status: NEW → ASSIGNED
(Reporter)

Comment 2

4 years ago
(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.
(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.
(Assignee)

Updated

4 years ago
Depends on: 1094858
(Assignee)

Comment 4

4 years ago
Created attachment 8518993 [details] [diff] [review]
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
Attachment #8518993 - Flags: review?(glob)
Attachment #8518993 - Flags: review?(glob) → review?(dylan)
Comment on attachment 8518993 [details] [diff] [review]
1089805_1.patch

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

r=dylan
Attachment #8518993 - Flags: review?(dylan) → review+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   c2533c1..2d223dc  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

4 years ago
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":"emorley@mozilla.com"},"is_private":0,"text":"Testing for bug 1062305."}],"id":"1069823","whiteboard":"","assigned_to":{"name":"eflores@mozilla.com"},"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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 8

4 years ago
(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":
> "emorley@mozilla.com"},"is_private":0,"text":"Testing for bug
> 1062305."}],"id":"1069823","whiteboard":"","assigned_to":{"name":
> "eflores@mozilla.com"},"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
(Reporter)

Comment 9

4 years ago
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?
(Reporter)

Comment 10

4 years ago
Created attachment 8521523 [details]
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?
(Reporter)

Comment 11

4 years ago
Created attachment 8521525 [details]
Curl POC

(Sorry fixed typo)
Attachment #8521523 - Attachment is obsolete: true
(Reporter)

Comment 12

4 years ago
$ ./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."}
(Assignee)

Comment 13

4 years ago
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
Component: Extensions: BzAPI Compatibility → Infrastructure
QA Contact: mcote
I'm getting the same discrepancy when going directly to a web head, so it's not the load balancers.
(Reporter)

Comment 15

4 years ago
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.
(Assignee)

Comment 16

4 years ago
(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
Flags: needinfo?(klibby)
(Reporter)

Comment 17

4 years ago
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
(Reporter)

Comment 18

4 years ago
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
Summary: BzAPI compatibility layer returns HTTP 200 when a bug update failed → BzAPI compat layer & native REST API incorrectly return HTTP 200 when using "Accept-Encoding: gzip"
this looks like bug 1036802
(Reporter)

Comment 20

4 years ago
(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
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Component: Infrastructure → Extensions: BzAPI Compatibility
Flags: needinfo?(klibby)
QA Contact: mcote
Resolution: --- → FIXED
Summary: BzAPI compat layer & native REST API incorrectly return HTTP 200 when using "Accept-Encoding: gzip" → BzAPI compat layer generates HTTP 404 errors for cases where legacy BzAPI used HTTP 400
(Reporter)

Updated

4 years ago
Blocks: 1098342
You need to log in before you can comment on or make changes to this bug.