638945 - Unexpected server error while validating add-on

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Developer Pages, defect, P2)

Description

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: The XPI causing the server error

This was reported to the mailing list and I could reproduce it on preview. The attached XPI is causing a server error in the validation step.

Comment 1

[Wil Clouser [:clouserw]]

Is server error the actual error message?  Because that doesn't sound like a validator problem

Comment 2

[Jorge Villalobos [:jorgev] (he/him)]

Attachment: Screenshot of server error

Here's what the developer sent us. I see the same thing on preview.

Comment 3

[Wil Clouser [:clouserw]]

-> kumar, I'd guess.

Comment 4

[Kumar McMillan [:kumar]]

I'm not sure why this error is occurring.  I can't reproduce it locally so it's only happening on preview.  The exception is that the spidermonkey version is too old but we have it configured with the right path.  Maybe the configured path is not the right one?  Bug 639584 will hopefully get us closer to solving it.  

For reference, here is the exception: 

Traceback (most recent call last):
  File "/data/amo_python/www/preview/zamboni/apps/devhub/tasks.py", line 27, in validator
    result = _validator(upload.path)
  File "/data/amo_python/www/preview/zamboni/apps/devhub/tasks.py", line 70, in _validator
    spidermonkey=settings.SPIDERMONKEY)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/validate.py", line 37, in validate
    validator.submain.prepare_package(bundle, path, expectation)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/submain.py", line 52, in prepare_package
    output = test_package(err, package, path, expectation)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/submain.py", line 140, in test_package
    return test_inner_package(err, package_contents, package)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/submain.py", line 203, in test_inner_package
    test_func(err, package_contents, package)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/content.py", line 166, in test_packed_packages
    file_data)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/scripting.py", line 35, in test_js_file
    errorbundle=err)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/scripting.py", line 257, in _get_tree
    raise RuntimeError("Spidermonkey version too old; "
RuntimeError: Spidermonkey version too old; 1.8pre+ required

Comment 5

[Kumar McMillan [:kumar]]

When the error change lands on preview (~10 min) I'll test it again.  https://github.com/jbalogh/zamboni-lib/commit/2e30f062eb2076445bfddd5c3ff82b39b8448948

Comment 6

[Kumar McMillan [:kumar]]

The code updates and now we can see the actual error.  I'm not sure exactly what's going on.  Matt, do you have any ideas?

RuntimeError: Spidermonkey version too old; 1.8pre+ required; error='ReferenceError: invalid assignment left-hand side'; spidermonkey='/data/bin/tracemonkey/tracemonkey'




Traceback (most recent call last):
  File "/data/amo_python/www/preview/zamboni/apps/devhub/tasks.py", line 27, in validator
    result = _validator(upload.path)
  File "/data/amo_python/www/preview/zamboni/apps/devhub/tasks.py", line 70, in _validator
    spidermonkey=settings.SPIDERMONKEY)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/validate.py", line 37, in validate
    validator.submain.prepare_package(bundle, path, expectation)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/submain.py", line 52, in prepare_package
    output = test_package(err, package, path, expectation)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/submain.py", line 137, in test_package
    return test_inner_package(err, package_contents, package)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/submain.py", line 223, in test_inner_package
    test_func(err, package_contents, xpi_package)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/content.py", line 157, in test_packed_packages
    file_data)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/scripting.py", line 35, in test_js_file
    err=err)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/javascript/spidermonkey.py", line 26, in get_tree
    shell if shell else SPIDERMONKEY_INSTALLATION)
  File "/data/amo_python/www/preview/zamboni/vendor/src/amo-validator/validator/testcases/javascript/spidermonkey.py", line 181, in _get_tree
    shell))
RuntimeError: Spidermonkey version too old; 1.8pre+ required; error='ReferenceError: invalid assignment left-hand side'; spidermonkey='/data/bin/tracemonkey/tracemonkey'

Comment 7

[Matt Basta [:basta]]

I'm able to reproduce the error on my VPS. The file that's causing the error, though, is rather innocuous. Additionally, a while back the new method for validating JS was pushed, meaning that all JS had to be pushed to a file and read in as a string rather than being embedded in a test file as a JSON-encoded string.

This means that the problem certainly isn't an issue with the test file that we pass to Spidermonkey. I'm inclined to believe that the issue is actually a problem with Reflect.parse(). Rather than throwing a syntax error or returning a parse tree that would have otherwise caused a runtime error had the code been eval-ed rather than parsed, it dumps the ReferenceError that we're seeing.

Can we get a JS guy in here to check this out?

Comment 8

[Matt Basta [:basta]]

The file in question:

http://pastebin.mozilla.org/1144472

Comment 9

[Matt Basta [:basta]]

Oh snap...is this happening because there's a hyphen in that variable's name? That would make it an expression, which can't be assigned a value.

I'll get to work on finding a way to lump that in with syntax errors.

Comment 10

[Dave Herman [:dherman]]

That there's some bogus JS. :)

The programmer tried to use an identifier called |video-sidebar| but that's not a valid identifier. Instead, it lexes as three tokens: |video|, |-|, and |sidebar|, which in turn parses as a subtraction expression. But you can't use a subtraction expression on the left-hand side of an assignment, and it looks like that's not what the programmer intended.

Dave

Comment 11

[Dave Herman [:dherman]]

BTW, you can tell that the ReferenceError is a compile-time error, not a runtime error, because if you try eval'ing code that does something at runtime before it gets to the expression, you get the ReferenceError without it ever performing the runtime action:

    js> eval("print('hello world'); x-y = 42")
    typein:2: ReferenceError: invalid assignment left-hand side:
    typein:2: print('hello world'); x-y = 42
    typein:2: ..........................^

So Reflect.parse() is throwing the same compile-time error that the normal SpiderMonkey compiler throws for this case. This means you'll want to make sure to check for ReferenceErrors in addition to SyntaxErrors in your validator.

Dave

Comment 12

[Heather Arthur [:harth]]

Attachment: another addon with validation server error

I experienced this today with the attached addon. Also, my previous files can't complete validation, showing this error message:

"Error: Validation task could not complete or completed with errors"

Comment 13

[Matt Basta [:basta]]

Fixed:

https://github.com/mozilla/amo-validator/pull/6

Comment 14

[Heather Arthur [:harth]]

Any word on when this could be in production, or should I try to get my xpi uploaded by someone manually?

Comment 15

[Matt Basta [:basta]]

Give it at least a week to make it to the launch on Thursday.

The error that you're seeing isn't actually the result of this bug, it's the result of bug 641037. That bug was also fixed and will launch in production at the same time that this bug launches.

Comment 16

[krupa raj[:krupa]]

verified fixed @ https://addons.allizom.org/en-US/developers/upload/cd4bebf913fe4105abdd58c9e70af34c

Comment 17

[krupa raj[:krupa]]

Attachment: post-fix screenshot

Comment 18

[Kris Maglione [:kmag]]

For what it's worth, the broken code was almost certainly caused by bug 625054.

Comment 19

[Matt Basta [:basta]]

Ehhhhhhh.....Don't think so. Nothing was broken, per-se, it was a matter of having multiple errors from Spidermonkey with the same exception type. You sure you posted to the right bug?

Comment 20

[Kris Maglione [:kmag]]

I meant the code posted in comment 8.

Comment 21

[Michael Kraft [:morac]]

Is this fixed on the production server?  

I'm currently running into the same exact error when trying to upload a new version of my Session Manager add-on.  I'm even failing when uploading a nearly exact copy of a version I already uploaded (only change is the version number).

The only way I can get validation to pass is to remove all the locales save en-US.  That shrinks the uncompressed version down from 1,295 KB to 678 KB, so I don't know if it's the size causing the problem or the locales.

Comment 22

[Jorge Villalobos [:jorgev] (he/him)]

The push to production will happen later today. I recommend you try again tomorrow.

Comment 23

[Michael Kraft [:morac]]

Okay thanks.

I actually just found the thread over at
https://forums.mozilla.org/addons/viewtopic.php?f=20&t=2637 and tested on
staging server and it works so I'll try again tomorrow as recommended.