Change Details

We should not log the `aborted` requests as `PagePreviewsAPIFailure` because that makes [[https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=now-30m&to=now|API Failures]] chart unusable. After deployment of T197700, the chart went from <10 events per minute to 200-400 events per minute and the graph doesn't present API failures. The noise generated by aborted requests is too big. The `abort` action should not increment the `PagePreviewsApiFailure` because it is not a request failure. ####Developer notes The [[ https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/actions.js#L154 | catch logic ]] in actions.js is incorrect. `result` parameter will be always undefined as the only place that causes `catch()` statement to be executed is this [[ https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/actions.js#L139| this throw ]]. Current logic assumes that every time we land in `catch((err, result) => { ... })` scope is not a network issue (because of the fact that result is always not defined) and it shows the generic page preview. Every fetch exception causes `FETCH_COMPLETE` state, which triggers the generic preview state. Because of that, we cannot: - determine if this is an offline issue and not show the page preview - it's not possible to not call `FETCH_COMPLETE` when the request was aborted #####Bonus points: [[https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/gateway/rest.js#L67|Rest gateway]] shouldn't know how to handle not found issues ####Acceptance criteria: [x] If I go to a page with Page Previews on, change to offline mode in the networking tab, hover a link - it should not show the Page Preview [x] When a request abort happens, Page Previews should not switch into FETCH_COMPLETE state [x] When a request is aborted, the `counter.PagePreviewsApiFailure` should not be incremented [x] when the server (both Mediawiki and Restbase) returns 4xx or 5xx error the Page Preview should be visible (the generic preview) [] POST Deployment: [[https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=now-30m&to=now|API Failures]] graph should get back to normal (less than 10 events per minute) = QA * Test on staging. * Drop connection, no page preview should show on any link * Restore connection, page previews should show on all links * When I hover over "List of D. C. Thomson & Co. Ltd publications" on http://reading-web-staging.wmflabs.org/wiki/Special:AllPages I should see an error (no lead section) * If page doesn't exist, a broken page preview will show e.g. Timed test 2 * If a page preview summary request results in a 500 error then a broken page preview will show (not sure how to test this one without changing code, but JR checked this manually)

We should not log the `aborted` requests as `PagePreviewsAPIFailure` because that makes [[https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=now-30m&to=now|API Failures]] chart unusable. After deployment of T197700, the chart went from <10 events per minute to 200-400 events per minute and the graph doesn't present API failures. The noise generated by aborted requests is too big. The `abort` action should not increment the `PagePreviewsApiFailure` because it is not a request failure. ####Developer notes The [[ https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/actions.js#L154 | catch logic ]] in actions.js is incorrect. `result` parameter will be always undefined as the only place that causes `catch()` statement to be executed is this [[ https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/actions.js#L139| this throw ]]. Current logic assumes that every time we land in `catch((err, result) => { ... })` scope is not a network issue (because of the fact that result is always not defined) and it shows the generic page preview. Every fetch exception causes `FETCH_COMPLETE` state, which triggers the generic preview state. Because of that, we cannot: - determine if this is an offline issue and not show the page preview - it's not possible to not call `FETCH_COMPLETE` when the request was aborted #####Bonus points: [[https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/gateway/rest.js#L67|Rest gateway]] shouldn't know how to handle not found issues ####Acceptance criteria: [x] If I go to a page with Page Previews on, change to offline mode in the networking tab, hover a link - it should not show the Page Preview [x] When a request abort happens, Page Previews should not switch into FETCH_COMPLETE state [x] When a request is aborted, the `counter.PagePreviewsApiFailure` should not be incremented [x] when the server (both Mediawiki and Restbase) returns 4xx or 5xx error the Page Preview should be visible (the generic preview) NOTE: This last AC can be verified after the MediaWiki train deployment on Thursday, 26th. [] Post-deployment: The [[https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=now-30m&to=now|API Failures]] graph should get back to normal. = QA * Test on staging. * Drop connection, no page preview should show on any link * Restore connection, page previews should show on all links * When I hover over "List of D. C. Thomson & Co. Ltd publications" on http://reading-web-staging.wmflabs.org/wiki/Special:AllPages I should see an error (no lead section) * If page doesn't exist, a broken page preview will show e.g. Timed test 2 * If a page preview summary request results in a 500 error then a broken page preview will show (not sure how to test this one without changing code, but JR checked this manually)

We should not log the `aborted` requests as `PagePreviewsAPIFailure` because that makes [[https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=now-30m&to=now|API Failures]] chart unusable. After deployment of T197700, the chart went from <10 events per minute to 200-400 events per minute and the graph doesn't present API failures. The noise generated by aborted requests is too big. The `abort` action should not increment the `PagePreviewsApiFailure` because it is not a request failure. ####Developer notes The [[ https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/actions.js#L154 | catch logic ]] in actions.js is incorrect. `result` parameter will be always undefined as the only place that causes `catch()` statement to be executed is this [[ https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/actions.js#L139| this throw ]]. Current logic assumes that every time we land in `catch((err, result) => { ... })` scope is not a network issue (because of the fact that result is always not defined) and it shows the generic page preview. Every fetch exception causes `FETCH_COMPLETE` state, which triggers the generic preview state. Because of that, we cannot: - determine if this is an offline issue and not show the page preview - it's not possible to not call `FETCH_COMPLETE` when the request was aborted #####Bonus points: [[https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/gateway/rest.js#L67|Rest gateway]] shouldn't know how to handle not found issues ####Acceptance criteria: [x] If I go to a page with Page Previews on, change to offline mode in the networking tab, hover a link - it should not show the Page Preview [x] When a request abort happens, Page Previews should not switch into FETCH_COMPLETE state [x] When a request is aborted, the `counter.PagePreviewsApiFailure` should not be incremented [x] when the server (both Mediawiki and Restbase) returns 4xx or 5xx error the Page Preview should be visible (the generic preview) NOTE: This last AC can be verified after the MediaWiki train deployment on Thursday, 26th. [] POST Dost-deployment: The [[https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=now-30m&to=now|API Failures]] graph should get back to normal (less than 10 events per minute). = QA * Test on staging. * Drop connection, no page preview should show on any link * Restore connection, page previews should show on all links * When I hover over "List of D. C. Thomson & Co. Ltd publications" on http://reading-web-staging.wmflabs.org/wiki/Special:AllPages I should see an error (no lead section) * If page doesn't exist, a broken page preview will show e.g. Timed test 2 * If a page preview summary request results in a 500 error then a broken page preview will show (not sure how to test this one without changing code, but JR checked this manually)