servo / servo

Servo, the embeddable, independent, memory-safe, modular, parallel web rendering engine
https://servo.org
Mozilla Public License 2.0
28.24k stars 3.02k forks source link

Line-number-less syntax errors on WPT xhr/overridemimetype-blob.html #24948

Open pshaughn opened 4 years ago

pshaughn commented 4 years ago

This test isn't using iframes, javascript: urls, eval, or anything else that would cause Javascript to be run without associated line numbers, and yet "The string did not match the expected pattern." (the SyntaxError message) appears in the test log with no Javascript trace. This isn't the only test I've seen that gets that error and no trace, but in the ones I saw before, there was more of a reason to think the line number might reasonably be absent.

I don't know how the WPT test harness actually works, so the line number thing could possibly be a tests problem and not a Servo problem. The syntax error happening at all is still a Servo bug if so.

pshaughn commented 4 years ago

xhr/overridemimetype-invalidmimetype.html does it too

jdm commented 4 years ago

In this particular case, the syntax error is triggered by returning Err::SyntaxError from a fallible DOM method (overrideMimeType), which is automatically converted into a JS exception by the WebIDL code generator:

        let result: Result<(), Error> = this.OverrideMimeType(arg0);
        let result = match result {
            Ok(result) => result,
            Err(e) => {
                throw_dom_exception(cx, &this.global(), e);
                return false;
            },
        };

I checked in a debugger and we don't end up in report_pending_exception, so the lack of meaningful output comes from the test harness catching our exception and failing to extract useful information from it: https://github.com/servo/servo/blob/69c7595a573db81798616ca5326457d8560a5377/tests/wpt/web-platform-tests/resources/testharness.js#L1989-L1991

There's code to capture a JS stack already in throw_dom_exception (but disabled by default, since it has big performance implications), but the harness would need to find a stack property on the DOMException interface in its current setup to provide anything useful.