Page MenuHomePhabricator

Display a sane error if the CN banner name is invalid, instead of a cryptic «BannerDataException»
Closed, ResolvedPublicPRODUCTION ERROR

Description

Encountered
[WZtMzQpAMFMAAFSENMYAAABT] 2017-08-21 21:12:45: Критичний виняток типу «BannerDataException»
[WZtNDQpAAEMAAJN7L4sAAABE] 2017-08-21 21:13:50: Критичний виняток типу «BannerDataException»
[WZtNaApAMFEAAH29CTAAAABA] 2017-08-21 21:15:21: Критичний виняток типу «BannerDataException»
[WZtOJwpAMFUAAEgb798AAAAR] 2017-08-21 21:18:32: Критичний виняток типу «BannerDataException»
[WZtXMwpAIDUAAFCNohsAAAAY] 2017-08-21 21:57:08: Критичний виняток типу «BannerDataException»
when trying to clone https://meta.wikimedia.org/wiki/Special:CentralNoticeBanners/edit/Community_banner_template with name of WikiEncuentro-Cali-2017 and desc of [[CentralNotice/Request/WikiEncuentro Cali 2017]].

Error

message
BannerDataException: Banner name must be in format /^[A-Za-z0-9_]+$/
exception.trace
#0 /srv/mediawiki/php-1.35.0-wmf.16/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(922): Banner->cloneBanner(string, User, string)
#1 /srv/mediawiki/php-1.35.0-wmf.16/includes/htmlform/HTMLForm.php(694): SpecialCentralNoticeBanners->processEditBanner(array, CentralNoticeHtmlForm)
#2 /srv/mediawiki/php-1.35.0-wmf.16/includes/htmlform/HTMLForm.php(586): HTMLForm->trySubmit()
#3 /srv/mediawiki/php-1.35.0-wmf.16/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(452): HTMLForm->tryAuthorizedSubmit()
#4 /srv/mediawiki/php-1.35.0-wmf.16/extensions/CentralNotice/includes/specials/SpecialCentralNoticeBanners.php(90): SpecialCentralNoticeBanners->showBannerEditor()
#5 /srv/mediawiki/php-1.35.0-wmf.16/includes/specialpage/SpecialPage.php(575): SpecialCentralNoticeBanners->execute(string)
#6 /srv/mediawiki/php-1.35.0-wmf.16/includes/specialpage/SpecialPageFactory.php(611): SpecialPage->run(string)
#7 /srv/mediawiki/php-1.35.0-wmf.16/includes/MediaWiki.php(298): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#8 /srv/mediawiki/php-1.35.0-wmf.16/includes/MediaWiki.php(967): MediaWiki->performRequest()
#9 /srv/mediawiki/php-1.35.0-wmf.16/includes/MediaWiki.php(530): MediaWiki->main()
#10 /srv/mediawiki/php-1.35.0-wmf.16/index.php(46): MediaWiki->run()
#11 /srv/mediawiki/w/index.php(3): require(string)
#12 {main}

Event Timeline

Worked for the name of [[https://meta.wikimedia.org/wiki/Special:CentralNoticeBanners/Edit/WikiEncuentro_Cali_2017|WikiEncuentro_Cali_2017]] (though it took over 5s to be done). Are hyphens the culprit or just a coincidence?

@Base: Why was this tagged as SRE ? Removing...

Are hyphens the culprit or just a coincidence?

Yes: [WZtXMwpAIDUAAFCNohsAAAAY] /wiki/Special:CentralNoticeBanners/edit/Community_banner_template BannerDataException from line 1069 of /srv/mediawiki/php-1.30.0-wmf.14/extensions/CentralNotice/includes/Banner.php: Banner name must be in format /^[A-Za-z0-9_

So maybe this task should be named "Display a sane error if the banner name is bad".

(Operations as a blind choice for someone to sort out errors, and CentralAuth was a typo, I was sure I selected CentralNotice)

Yes, given the hyphen is confirmed to be behind this rather than some bug, the suggested name for the task looks good to me too :)

Aklapper renamed this task from «BannerDataException» when trying to clone a banner to Display a sane error if the CN banner name is invalid, instead of a cryptic «BannerDataException» (when trying to clone a banner).Aug 22 2017, 10:36 AM

The error display was resolved in https://phabricator.wikimedia.org/T149240, in that it now says "You do not have the correct permissions to perform the requested action or the action itself was invalid." as opposed to a full stack trace. However, I wonder if it's worth displaying the specific error message when a user inputs an invalid format.

Change 533253 had a related patch set uploaded (by Mepps; owner: Mepps):
[mediawiki/extensions/CentralNotice@master] Use specific error message when banner name missing/invalid

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

Change 533253 merged by jenkins-bot:
[mediawiki/extensions/CentralNotice@master] Use specific error message when banner name missing/invalid

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

Change 611713 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/extensions/CentralNotice@master] Validate banner name on SpecialCentralNoticeBanners for delete and clone

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

Krinkle renamed this task from Display a sane error if the CN banner name is invalid, instead of a cryptic «BannerDataException» (when trying to clone a banner) to Display a sane error if the CN banner name is invalid, instead of a cryptic «BannerDataException».Sep 10 2020, 6:45 PM
Krinkle changed the subtype of this task from "Task" to "Production Error".
Krinkle updated the task description. (Show Details)
Krinkle moved this task from Untriaged to Feb2020/1.35-wmf.18 on the Wikimedia-production-error board.

Would be nice to get a feedback if the provided patch set is the right way to fix the issue or not.
That would allow to unblock the task by merging the patch set. Or abandon the patch set and make it free for another try from another developer, but a task with patch set maybe not getting much attention for other solutions.

I'm not sure if you mean me for testing, but for me it doesn't work yet.
(see screenshot)

Screenshot_2021-03-01 Internal error - Meta.png (205×1 px, 10 KB)

Change 611713 abandoned by Umherirrender:
[mediawiki/extensions/CentralNotice@master] Validate banner name on SpecialCentralNoticeBanners for delete and clone

Reason:
Seems not the right way to do it, unblock the task for other developer to find a better way.

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

DStrine moved this task from Q4 2020-2021 to Current Sprint on the Fundraising-Backlog board.

Change 611713 restored by AndyRussG:

[mediawiki/extensions/CentralNotice@master] Validate banner name on SpecialCentralNoticeBanners for delete and clone

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

@Umherirrender the patch you sent looks fine. Many, many apologies to all that we hadn't gotten to this sooner. I've restored the patch, and we'll merge it shortly. Thanks so much and apologies again!!

Change 611713 merged by jenkins-bot:

[mediawiki/extensions/CentralNotice@master] Validate banner name on SpecialCentralNoticeBanners for delete and clone

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

Umherirrender claimed this task.

@Umherirrender the patch you sent looks fine. Many, many apologies to all that we hadn't gotten to this sooner. I've restored the patch, and we'll merge it shortly. Thanks so much and apologies again!!

Thanks!