Page MenuHomePhabricator

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


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

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

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

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

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

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.

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.

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