708056 - Show a readable validation error when it times out

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Add-on Validation, defect, P2)

Description

[Kumar McMillan [:kumar]]

When validation times out, a RuntimeError exception gets raised. We should catch this and return readable message to the user so they know what happened.

Current hotfix for timeouts is here (thanks Matt for the signal idea)
https://github.com/mozilla/amo-validator/compare/60eb9218637095781c3ea13099dfccd7aeb747e4...84cf737f676e9135536f209a7c7e2fe889f528f1

Comment 1

[Matt Basta [:basta]]

Working on this. The only concern I have is that we can get RuntimeErrors during JS validation, namely when Spidermonkey can't be accessed or is too old.

https://github.com/mozilla/amo-validator/blob/master/validator/testcases/javascript/spidermonkey.py

Would it be wise to create a new exception type? Perhaps SpidermonkeyError?

Comment 2

[Kumar McMillan [:kumar]]

Yes, please change my quick and dirty RuntimeError('time out') hack to ValidationTimeout() or something better. It was not meant as a permanent fix.

Comment 3

[Matt Basta [:basta]]

Also in https://github.com/mozilla/amo-validator/pull/98

Comment 4

[Kumar McMillan [:kumar]]

Hi Matt, thanks for the patch. Not sure if you saw my other comment on the pull, can you make separate pull requests per bug? That makes it way easier to prioritize when there are a lot of bugs to r and merge in.

Comment 5

[Matt Basta [:basta]]

Yeah, no problem. I got your message after I'd pushed this one. Do you want me to go back and rebase this onto its own branch? The only reason I was clumping these was because the Jetpack submodule makes a load of tests fail (paths changed in the recent SDK versions).

Comment 6

[Kumar McMillan [:kumar]]

don't worry about rebasing

Comment 7

[Matt Basta [:basta]]

Merged

https://github.com/mozilla/amo-validator/commit/3fb6e7529c9148ed671aa943c56f4567637258c3

Comment 8

[Kumar McMillan [:kumar]]

Applied to Zamboni in https://github.com/mozilla/zamboni/commit/8a4b5ad