Page MenuHomePhabricator

CentralNotice: Relay banner loading issues in beacon/impression
Closed, ResolvedPublic2 Story Points

Description

We're not currently recording when there are issues with banner loading (as happened in T144952). Since these are not successful impressions, we need to stop recording them as such.

Related Objects

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 25 2016, 6:38 PM

Change 318159 had a related patch set uploaded (by AndyRussG):
Handle banner loader errors on client

https://gerrit.wikimedia.org/r/318159

Change 318159 merged by jenkins-bot:
Handle banner loader errors on client

https://gerrit.wikimedia.org/r/318159

Change 320317 had a related patch set uploaded (by AndyRussG):
Handle banner loader errors on client

https://gerrit.wikimedia.org/r/320317

Change 320317 merged by jenkins-bot:
Handle banner loader errors on client

https://gerrit.wikimedia.org/r/320317

Mentioned in SAL (#wikimedia-operations) [2016-11-08T00:12:43Z] <dereckson@tin> Synchronized php-1.29.0-wmf.1/extensions/CentralNotice/: Handle banner loader errors on client (T149107) (duration: 00m 49s)

Sadly, this isn't working as expected. Since this 2016-11-09 (the day after the patch for this was deployed) we have 93 errors sent back via beacon/impression. However, none of them are the missing banner message problem, which we still see PHP-side in the debug log and were specifically trying to catch.

OccurrenceserrorMsgBanner(s)
83Invalid banner name supplied.B16WMDE_06_161108_ctrl, B16WMDE_06_161108_ctrl_b, B16WMDE_06_161108_var, B16WMDE_07_161111_top_var, B16WMDE_en01_161109_ctrl, B16WMDE_en01_161109_webelieve
5error:2016WishlistSurvey, WAM_2016
4Empty bannerB16WMDE_mob_01_161109_var
1error: TypeError: transport.send is not a functionB1617_1031_frFR_mob_p2_sm_dsn_exp

(Excluding one call to beacon/impression apparently due to someone testing mw.centralNotice.handleBannerLoaderError() from the JS console.)

The PHP-side log, however, does show occurrences of "Banner message key $bannerKey could not be found in $lang" over the same period. So, something's fishy...

Of the errors reported via beacon/impression, only the first seems potentially significant to me. The ones that start with "error: " were handled client-side via a rejection of the ajax promise. I think they could be due our JS trying to run on incompatible clients.

awight assigned this task to AndyRussG.Nov 21 2016, 5:27 PM
awight added a subscriber: awight.Nov 23 2016, 9:49 PM
This comment was removed by awight.

K got it! The explanation is pretty simple: this bug is only occurring for a short period after a banner has been created. After a banner is added to a campaign, we have to wait for the ResourceLoader cache to refresh before the ChoiceData with the new banner name is sent to users and the banner can be retrieved as part of a campaign.

This means that the log entries we see for missing banner content (fetched as i18n messages from MessageCache by banner loader) are almost certainly from banner previews. In such cases, beacon/impression is not called.

I tested that a missing banner does indeed trigger the handleBannerLoaderError() callback by modifying a banner name directly in the DB on the beta cluster. When the client tried to fetch the banner as part of a campaign, beacon/impression correctly relayed the error.

So... it seems that this does work as expected, after all! I'd suggest that we resolve this task. :) Thanks and apologies for not figuring this issue out sooner!! ;p

AndyRussG closed this task as Resolved.Dec 20 2016, 10:40 PM
mmodell removed a subscriber: awight.Jun 22 2017, 9:34 PM