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.
Description
Description
Details
Details
Related Objects
Related Objects
- Mentioned Here
- T144952: Banner not showing up on site
Event Timeline
Comment Actions
Change 318159 had a related patch set uploaded (by AndyRussG):
Handle banner loader errors on client
Comment Actions
Change 320317 had a related patch set uploaded (by AndyRussG):
Handle banner loader errors on client
Comment Actions
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)
Comment Actions
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.
| Occurrences | errorMsg | Banner(s) |
| 83 | Invalid 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 |
| 5 | error: | 2016WishlistSurvey, WAM_2016 |
| 4 | Empty banner | B16WMDE_mob_01_161109_var |
| 1 | error: TypeError: transport.send is not a function | B1617_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.
Comment Actions
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