1563824 - Expose 429 status/error code to proxy extensions

Status: VERIFIED FIXED

(Core :: Networking: HTTP, task, P1)

Comment 1

[Kershaw Chang [:kershaw]]

Attachment: Bug 1563824 - New error NS_ERROR_TOO_MANY_REQUESTS for 429 response

Comment 2

[Honza Bambas (:mayhemer)]

Kershaw, be aware you will have to format the js changes for m-c landing according the new linting rules with ./mach eslint --fix.

See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style/Formatting_JS_Code_With_Prettier_and_eslint

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c39d4a7790e5
New error NS_ERROR_TOO_MANY_REQUESTS for 429 response r=mayhemer

Comment 4

[Kershaw Chang [:kershaw]]

Comment on attachment 9076322 [details]
Bug 1563824 - New error NS_ERROR_TOO_MANY_REQUESTS for 429 response

Beta/Release Uplift Approval Request

• User impact if declined: Not showing http 429 response properly.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch just introduces a new error code and return it when receiving 429 response.
• String changes made/needed:

Comment 5

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/c39d4a7790e5

Comment 6

[Honza Bambas (:mayhemer)]

You should have asked release approval.

Comment 7

[Kershaw Chang [:kershaw]]

Comment on attachment 9076322 [details]
Bug 1563824 - New error NS_ERROR_TOO_MANY_REQUESTS for 429 response

Beta/Release Uplift Approval Request

• User impact if declined: Not showing http 429 response properly.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch just introduces a new error code and return it when receiving 429 response.
• String changes made/needed:

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 9076322 [details]
Bug 1563824 - New error NS_ERROR_TOO_MANY_REQUESTS for 429 response

This is already in 69.

Comment 9

[Alexandru Trif, Desktop QA [:atrif]]

I will mark this qe-verify- as per comment 4 and comment 7.

Comment 10

[Rebecca Billings [:rbillings]]

Please add repro steps with expected results for testing.

Comment 11

[Honza Bambas (:mayhemer)]

There is no way to test this bug manually easily. We have automated tests for this that pass. This is qe-.

Comment 12

[Rebecca Billings [:rbillings]]

Marking this bug as resolved fixed and will re-open if any issues occur during regular browsing using the proxy.

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9076322 [details]
Bug 1563824 - New error NS_ERROR_TOO_MANY_REQUESTS for 429 response

I wish the uplift request had more details. Why is 429 important? What happens if it's not exposed? Why is that bad?

Anyway, approved for 68.0.1 and 68.1esr.

Comment 14

[Honza Bambas (:mayhemer)]

(In reply to Julien Cristau [:jcristau] from comment #13)

Comment on attachment 9076322 [details]
Bug 1563824 - New error NS_ERROR_TOO_MANY_REQUESTS for 429 response

I wish the uplift request had more details. Why is 429 important?

This is a "too many requests sent to me" error code that a proxy notifies the client of a possible abuse.

What happens if it's not exposed?

We can't handle it gracefully and show the user a proper notification/error page explaining why his/her proxy stopped working - that the user is abusing the proxy over limits.

Why is that bad?

Anyway, approved for 68.0.1 and 68.1esr.

thanks.

Comment 15

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-release/rev/d71ed91e9422

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Per discussion with jcristau, we're uplifting this to 68.0.1esr also to maintain parity with the non-ESR 68.0.1 release and hopefully avoid some confusion.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

default (68.1esr): https://hg.mozilla.org/releases/mozilla-esr68/rev/7af424604cc50e52920fc01dfa04a451973d16bf
FIREFOX_ESR_68_0_X_RELBRANCH (68.0.1esr): https://hg.mozilla.org/releases/mozilla-esr68/rev/af344489909a920090b7030d61a32e3f1a26bd30