Page MenuHomePhabricator
Create Task
Maniphest T261580

Special:GlobalWatchlist breaks on API error
Closed, ResolvedPublic3 Estimated Story PointsBUG REPORT
Actions

Description

For example, try to load the global watchlist while you are not logged in on one of the watched sites. The API will return a notloggedin error, and the page will throw a GlobalWatchlist error, please check the console! alert that leaves the page in a broken state. A more reasonable approach would be to load the data from all the sites that work, and show a warning box listing the sites that didn't.

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Add handling for API failures when fetching watchlistmediawiki/extensions/GlobalWatchlistmaster+107 -49
Customize query in gerrit

Related Objects

Event Timeline

Tgr created this task.Aug 30 2020, 3:23 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 30 2020, 3:23 PM
Tgr added a comment.Aug 30 2020, 3:25 PM

(Also ideally the tool would use a method that does not depend on login cookies, such as a centralauthtoken request.)

Tgr edited projects, added MediaWiki-extensions-GlobalWatchlist; removed Global_watchlist.js.Aug 30 2020, 3:25 PM
Restricted Application added a project: User-DannyS712. · View Herald TranscriptAug 30 2020, 3:25 PM
DannyS712 changed the subtype of this task from "Task" to "Bug Report".Aug 30 2020, 5:44 PM
DannyS712 subscribed.
In T261580#6421590, @Tgr wrote:

(Also ideally the tool would use a method that does not depend on login cookies, such as a centralauthtoken request.)

Currently this uses the mw.ForeignApi, which, if CentralAuth is installed, should be using centralauthtoken - https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/CentralAuth/+/0d49a86ae7a870a5904a79f64bb32d76dac15ca7/modules/ext.centralauth.ForeignApi.js

gerritbot added a comment.Aug 30 2020, 5:55 PM

Change 623148 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/GlobalWatchlist@master] Add handling for API failures

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

gerritbot added a project: Patch-For-Review.Aug 30 2020, 5:55 PM
DannyS712 claimed this task.Aug 30 2020, 6:09 PM
DannyS712 moved this task from Backlog to In progress on the MediaWiki-extensions-GlobalWatchlist board.
DannyS712 set the point value for this task to 3.
Tgr added a comment.Aug 30 2020, 6:48 PM

Oh, right, I was testing it on Vagrant with no centralauth role, of course it has to use session cookies there.

gerritbot added a comment.Sep 13 2020, 5:10 PM

Change 623148 merged by jenkins-bot:
[mediawiki/extensions/GlobalWatchlist@master] Add handling for API failures when fetching watchlist

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

jenkins-bot mentioned this in rEGLW9e4a9c6d5023: Add handling for API failures when fetching watchlist.Sep 13 2020, 5:11 PM
DannyS712 removed a project: Patch-For-Review.Sep 13 2020, 5:13 PM

@Tgr I believe this is resolved, but since my local set up uses CentralAuth, can you check if it still errors out for you?

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.9; 2020-09-15).Sep 13 2020, 6:00 PM
Tgr added a comment.Sep 16 2020, 4:28 AM

I still get the same error. The useful part of the stack trace is

Error: GlobalWatchlistError: en.wiki.local.wmftest.net:8765:API.actuallyGetWatchlist #1 - notloggedin
    at GlobalWatchlistDebugger.error
    at GlobalWatchlistSiteDisplay.SiteBase.js.GlobalWatchlistSiteBase.error

The UI does seem to recover from the error now, though.

DannyS712 added a comment.EditedSep 16 2020, 4:31 AM
In T261580#6465083, @Tgr wrote:

I still get the same error. The useful part of the stack trace is

Error: GlobalWatchlistError: en.wiki.local.wmftest.net:8765:API.actuallyGetWatchlist #1 - notloggedin
    at GlobalWatchlistDebugger.error
    at GlobalWatchlistSiteDisplay.SiteBase.js.GlobalWatchlistSiteBase.error

The UI does seem to recover from the error now, though.

Okay, that was the goal (the UI recovering recovering, not the still having an error part) - since this should be used with centralauth, notloggedin shouldn't be an issue for the cross site functionality. Is it okay to close this as resolved?

Tgr added a comment.Sep 17 2020, 1:12 AM

That's your call. It would be nice to handle notloggedin and give the user a login link instead of a cryptic error message, but given the intended audience, might not be worth the effort.

DannyS712 closed this task as Resolved.Sep 17 2020, 1:20 AM
DannyS712 moved this task from In progress to Done on the MediaWiki-extensions-GlobalWatchlist board.
In T261580#6468949, @Tgr wrote:

That's your call. It would be nice to handle notloggedin and give the user a login link instead of a cryptic error message, but given the intended audience, might not be worth the effort.

If this gets reported in WMF prod I'll do that (as well as for any other error messages that are found) but until then this isn't a priority, sorry

DannyS712 set Final Story Points to 3.Sep 17 2020, 1:20 AM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits