Last modified: 2014-05-10 09:04:17 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T62936, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 60936 - Flow: unrecognized and unsupported URL actions throw fatal errors
Flow: unrecognized and unsupported URL actions throw fatal errors
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Flow (Other open bugs)
master
All All
: High normal (vote)
: ---
Assigned To: Kunal Mehta (Legoktm)
https://www.mediawiki.org/wiki/Talk:S...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-02-06 02:12 UTC by spage
Modified: 2014-05-10 09:04 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description spage 2014-02-06 02:12:33 UTC
Bug 60834 complains that ?action=info doesn't work on a Flow page.  Perhaps action=info should display useful information, see discussion at that bug.

The issue at hand is whether Flow should ignore unrecognized/unsupported actions, as it does now -- you just see the Flow board. That's misleading if someone follows a URL that used to do something useful before Flow was enabled on the page.

On a regular wiki page ?action=BAD results in
  No such action
  The action specified by the URL is invalid. You might have mistyped the URL, or followed an incorrect link. This might also indicate a bug in the software used by {{SITENAME}}. 

(enwiki customizes the second, <nosuchactiontext>, message).

Maybe Flow should respond similarly, with a <flow-nosuchactiontext> changed to "... is invalid for a Flow board. ..."
Comment 1 Kunal Mehta (Legoktm) 2014-02-06 03:43:51 UTC
I agree with not silently failing.

Some of the actions rather than failing should go to the relevant Flow-action. So for example, ?action=history should redirect the user to ?action=board-history.
Comment 2 Maryana Pinchuk 2014-02-06 20:00:40 UTC
+1 to Kunal – where possible, we should just drop the unsupported query string and take the user to the page (history, board, etc.). That's what mediawiki does as a fallback; that's what we should do, too.
Comment 3 Kunal Mehta (Legoktm) 2014-04-16 07:16:07 UTC
I don't know what's changed in the code since when this bug was filed, but unsupported actions are now throwing fatal errors as can be seen on the provided url. Adjusting priority accordingly.
Comment 4 spage 2014-04-23 18:49:03 UTC
We're OK with the current behavior (show unrecognized action message and display exception)
Comment 5 Kunal Mehta (Legoktm) 2014-04-24 03:02:28 UTC
No, displaying the exception is not ok. That means it's also going in the error logs, even though it's not a true exception.
Comment 6 spage 2014-04-24 05:23:54 UTC
Not quite :)  Flow's InvalidActionException returns false for MWException::isLoggable(), so nothing is logged. So unless this happens to a Flow engineer, the fingerprinted pink 
  [46a903b5] 2014-04-24 05:21:54: Fatal exception of type Flow\Exception\InvalidActionException

in production isn't very useful, since the 46a903b5 fingerprint isn't written anywhere.  Maybe Flow should log a backtrace to wfDebugLog( 'Flow', )

Bug 62176 "improve exception handling, reuse more core (edit)" is related.
Comment 7 Ori Livneh 2014-04-24 05:51:51 UTC
(In reply to spage from comment #6)
> So unless this happens to a Flow engineer

Flowgineer? Flowician? Flowologist?
Comment 8 Kunal Mehta (Legoktm) 2014-04-24 08:24:52 UTC
(In reply to spage from comment #6)
> Not quite :)  Flow's InvalidActionException returns false for
> MWException::isLoggable(), so nothing is logged. So unless this happens to a
> Flow engineer, the fingerprinted pink 
>   [46a903b5] 2014-04-24 05:21:54: Fatal exception of type
> Flow\Exception\InvalidActionException

Huh, I didn't realize that. I still think that displaying the standard exception info is wrong, and will upload a patchset for it in a moment.
Comment 9 Gerrit Notification Bot 2014-04-24 08:26:43 UTC
Change 129384 had a related patch set uploaded by Legoktm:
Catch and specially handle InvalidArgumentException

https://gerrit.wikimedia.org/r/129384
Comment 10 Gerrit Notification Bot 2014-05-05 12:51:17 UTC
Change 131455 had a related patch set uploaded by Matthias Mullie:
Catch and specially handle InvalidArgumentException

https://gerrit.wikimedia.org/r/131455
Comment 11 Gerrit Notification Bot 2014-05-05 23:42:51 UTC
Change 131455 merged by jenkins-bot:
Catch and specially handle InvalidArgumentException

https://gerrit.wikimedia.org/r/131455
Comment 12 Gerrit Notification Bot 2014-05-09 00:16:00 UTC
Change 129384 abandoned by Legoktm:
Catch and specially handle InvalidArgumentException

Reason:
I05d6be4a59db57c24bd9a11c499cb69f18feaa41  is a better implementation.

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

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links