Page MenuHomePhabricator

Unknown project should not cause an Internal Server Error
Closed, ResolvedPublic1 Estimated Story Points

Description

If you give the Admin stats interface a project name that doesn't exist, it returns a 500 internal server error:
http://tools.wmflabs.org/xtools-dev/adminstats/Blah

Event Timeline

kaldari triaged this task as Medium priority.May 25 2017, 6:29 PM
kaldari moved this task from New & TBD Tickets to Needs Discussion on the Community-Tech board.
kaldari set the point value for this task to 1.
kaldari moved this task from Needs Discussion to Up Next on the Community-Tech board.
Matthewrbowker moved this task from Backlog to Working on the XTools board.
Matthewrbowker subscribed.

I'll take a look

I do not think it is a good idea to throw an exception when a project does not exist. databasePrepare() in LabsHelper.php must change. Having some kind of error "pipeline" would also help notify users something is going wrong.

It looks like this happens in the Edit Counter interface as well. Put in a bogus project and you get a 500 error:
http://tools.wmflabs.org/xtools-dev/ec/dfgdfg/Kaldari

Yes, I've had a todo list item for a while. The expected behavior is to add a "flash notice" then redirect back to the form.

@Superyetkin Indeed. Error pipline is defined above, and there is also a TODO comment at https://phabricator.wikimedia.org/source/tool-xtools-rebirth/browse/master/src/AppBundle/Helper/LabsHelper.php;7afccb7ee0de515c06149549c420df77ce6a2bda$48

@kaldari Yes, every interface that uses databasePrepare() will have this problem.

Matthewrbowker renamed this task from Admin stats fatals if project isn't recognized to Unknown project should not cause an Internal Server Error.May 25 2017, 8:55 PM

Pull Request merged.