1854115 - The review checker sidebar remains blank on the pages where the OHTTP request limit is exceeded

Status: VERIFIED FIXED

(Firefox :: Shopping, defect, P3)

Description

[Peter Magyari (Desktop QA)]

Attachment: Screenshot 2023-09-20 at 14.31.41.png

Found in

• Firefox 119.0a1 (2023-09-19)

Affected versions

• Firefox 119.0a1 (2023-09-20)

Tested platforms

• Affected platforms: All

Preconditions

• browser.shopping.experience2023.enabled- true
• browser.shopping.experience2023.optedIn - 1
• toolkit.shopping.ohttpConfigURL - https://stage.ohttp-gateway.nonprod.webservices.mozgcp.net/ohttp-configs
• toolkit.shopping.ohttpRelayURL - https://mozilla-ohttp-dev.fastly-edge.com/

Steps to reproduce

1. Open Firefox.
2. Open 51 random product pages from amazon/walmart/bestbuy.
3. Open browser toolbox to the network monitor tab.
4. Select all of the tabs and reload them.

Expected result

• Review checker sidebar is populated accordingly on every open product page.

Actual result

• Since the OHTTP rate-limit is reached, 429 error(s) are thrown in the network monitor.
• Loading 51 product pages at once results in 2 pages not having the review checker sidebar populated. (2 POST events turn back with status-code:429)
• The 2 pages remain with a blank review checker sidebar until refreshed.

Regression range

• Not a regression.

Additional notes

• This also occurs when opening 50+ bookmarks at once or sending 50+ product page tabs to another device (through sync).

Comment 1

[:Gijs (he/him)]

Fred, do you have time to investigate this?

Comment 2

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

This seems like a pretty remote edge case to me. Not sure it really needs to be prioritized in 119. I'll ping Ania to get her input

Comment 3

[Ania]

Katie, in this state we would ideally want to display a sidebar error informing users that "too many requests have been made to the server, please wait a minute and try again".
Can you please lend us a hand with wordsmithing for it? We will use the standard sidebar error state styling.

Comment 4

[:Gijs (he/him)]

Actually, let me split my concern into a separate bug.

Comment 5

[Ania]

Discussed in the office hours:

• generic error displayed for now (ideally 119),
• custom message for this error in the future (lower priority)

Comment 6

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

Bumping to 121

Comment 7

[Fred Chasen [:fchasen]]

Attachment: Bug 1854115 - Handle Shopping OHTTP errors and not ok statuses. r=#shopping-reviewers

Wraps the calls to QueryInterface(Ci.nsIHttpChannel) in ohttpRequest in try blocks and rejects the request promise on errors.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5cb2c0551ff2
Handle Shopping OHTTP errors and not ok statuses. r=shopping-reviewers,Gijs

Comment 9

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on test_product.js

• backout: https://hg.mozilla.org/integration/autoland/rev/938e6fc607623834483a5ab95019520814f10ee8
• push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=5cb2c0551ff2ba188695ab6b49f2ac9dd75e3110
• failure log: https://treeherder.mozilla.org/logviewer?job_id=435659843&repo=autoland&lineNumber=4943

[task 2023-11-09T18:46:24.515Z] 18:46:24     INFO -  TEST-START | toolkit/components/shopping/test/xpcshell/test_product.js
[task 2023-11-09T18:46:25.676Z] 18:46:25  WARNING -  TEST-UNEXPECTED-FAIL | toolkit/components/shopping/test/xpcshell/test_product.js | xpcshell return code: 0
[task 2023-11-09T18:46:25.676Z] 18:46:25     INFO -  TEST-INFO took 1158ms
[task 2023-11-09T18:46:25.677Z] 18:46:25     INFO -  >>>>>>>
[task 2023-11-09T18:46:25.677Z] 18:46:25     INFO -  (xpcshell/head.js) | test MAIN run_test pending (1)
[task 2023-11-09T18:46:25.677Z] 18:46:25     INFO -  (xpcshell/head.js) | test run_next_test 0 pending (2)
[task 2023-11-09T18:46:25.678Z] 18:46:25     INFO -  (xpcshell/head.js) | test MAIN run_test finished (2)
[task 2023-11-09T18:46:25.679Z] 18:46:25     INFO -  running event loop
[task 2023-11-09T18:46:25.680Z] 18:46:25     INFO -  toolkit/components/shopping/test/xpcshell/test_product.js | Starting test_product_requestAnalysis
[task 2023-11-09T18:46:25.680Z] 18:46:25     INFO -  (xpcshell/head.js) | test test_product_requestAnalysis pending (2)
[task 2023-11-09T18:46:25.681Z] 18:46:25     INFO -  TEST-PASS | toolkit/components/shopping/test/xpcshell/test_product.js | test_product_requestAnalysis - [test_product_requestAnalysis : 258] Should recognize a valid product. - true == true

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b46da975c447
Handle Shopping OHTTP errors and not ok statuses. r=shopping-reviewers,Gijs

Comment 11

[Atila Butkovits]

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

Comment 12

[Peter Magyari (Desktop QA)]

Verified fixed using Firefox Nightly 121.0a1 (20231113215623) on MacOS 11, Windows 10 and Ubuntu 20.04.
Loading 51 product pages at once results in 2 pages returning the "No info available right now" message in the review checker toolbar.
(2 POST events with status-code:429)

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:fchasen, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox120 to wontfix.

For more information, please visit BugBot documentation.

Comment 14

[Fred Chasen [:fchasen]]

Comment on attachment 9362249 [details]
Bug 1854115 - Handle Shopping OHTTP errors and not ok statuses. r=#shopping-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: If a user hits the maximum number of allowed Fakespot API requests via OHTTP, the sidebar will remain in a loading state instead of showing an error.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: - Load 51 Amazon product pages at once with the Shopping Sidebar active.

• Open a new Amazon product page.

New page should show the "No info available right now" message in the Shopping Sidebar, and not remain in the loading state.

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Change is not risky as it adds better error handling and only users who are being request limited by the the Fakespot API will be affected by this update.
• String changes made/needed:
• Is Android affected?: No

Comment 15

[Dianna Smith [:diannaS]]

Comment on attachment 9362249 [details]
Bug 1854115 - Handle Shopping OHTTP errors and not ok statuses. r=#shopping-reviewers

changing flag to release for consideration for the next dot release

Comment 16

[Dianna Smith [:diannaS]]

Comment on attachment 9362249 [details]
Bug 1854115 - Handle Shopping OHTTP errors and not ok statuses. r=#shopping-reviewers

Approved for 120.0.1 dot release

Comment 17

[Pulsebot]

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

Comment 18

[Dianna Smith [:diannaS]]

Comment on attachment 9362249 [details]
Bug 1854115 - Handle Shopping OHTTP errors and not ok statuses. r=#shopping-reviewers

Backed out changeset cd64322134ff (bug 1854115) for causing xpc shells failures on test_product.js
depends on Bug 1859055
will ride 121 train

backout https://hg.mozilla.org/releases/mozilla-release/rev/04325acc98c1

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/04325acc98c1

Comment 20

[Rares Doghi, Desktop QA]

Updating the main status flag.