1601583 - Provide context for inline JavaScript SyntaxError "missing ) after argument list"

Status: NEW

(Core :: JavaScript Engine, enhancement, P3)

Description

[Claas Augner [:claas]]

Background

JavaScript errors can be captured using services like Sentry. For example, clicking the button in the following HTML snippet
(JSFiddle) ...

<button type="button" onclick="console.log('Hello World!)'">Hello world</button>

... causes a SyntaxError (because the closing single quote is misplaced) with the message:

missing ) after argument list

Problem

Unfortunately the resulting Error object does not contain any useful stacktrace (because the JavaScript is defined inline). This makes it very hard to locate the error, especially on (legacy) pages with many event handlers defined this way.

Here is an example of such a SyntaxError captured by Raven/Sentry (URLs redacted):

stdClass Object
(
    [project] => raven
    [logger] => javascript
    [platform] => javascript
    [request] => stdClass Object
        (
            [headers] => stdClass Object
                (
                    [User-Agent] => Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:70.0) Gecko/20100101 Firefox/70.0
                    [Referer] => https://***
                )

            [url] => https://***
        )

    [exception] => stdClass Object
        (
            [values] => Array
                (
                    [0] => stdClass Object
                        (
                            [type] => SyntaxError
                            [value] => missing ) after argument list
                            [stacktrace] => stdClass Object
                                (
                                    [frames] => Array
                                        (
                                            [0] => stdClass Object
                                                (
                                                    [filename] => https://***
                                                    [lineno] => 2
                                                    [function] => ?
                                                    [in_app] => 1
                                                )

                                        )

                                )

                        )

                )

            [mechanism] => stdClass Object
                (
                    [type] => onerror
                    [handled] => 
                )

        )

    [transaction] => https://***
    [extra] => stdClass Object
        (
            [session:duration] => 4840
        )

    [event_id] => 0902a74a5fe042e39f40edd425e796ae

)

</details>

Solution

It would be extremely useful to extend the SyntaxError message to contain an extract of the source code that caused the SyntaxError, e.g.:

missing ) after argument list near: console.log('Hello World!)'

Comment 1

[Jason Orendorff [:jorendorff]]

Unfortunately the resulting Error object does not contain any useful stacktrace (because the JavaScript is defined inline). This makes it very hard to locate the error, especially on (legacy) pages with many event handlers defined this way.

Are the filename and line number correct?

Putting some source code into .message is a nice hack, but it also has downsides. It could be confusing; it could break web sites that (unwisely) depend on the text in .message (this really happens). And it's still pretty manual. Something machine-readable would be better, if we can swing it.

:jlast, the DevTools Console has the same problem with this JSFiddle page: if you open the console, then click the Hello World button, you get an error in the console, but then clicking on the location takes you an irrelevant line of code.

Is this a known issue?

Comment 2

[Jason Laster [:jlast]]

Hmm, this might be relevant

https://bugzilla.mozilla.org/show_bug.cgi?id=1407066

By the way, I found this with https://firebugs.dev/#bugs/DevTools/Console, which i find to be a faster for fuzzy matching but titles.

Comment 3

[Nicolas Chevobbe [:nchevobbe]]

jsFiddle opens some source in the debugger (a 404 response somehow ?)
Glitch opens a view source tab: https://bolder-elf.glitch.me/

Brian, this looks similar to the issues you fixed this year. Would you know what's happening? Is this a regression?

Comment 4

[Claas Augner [:claas]]

First of all, thanks a lot for taking the time to respond so quickly.

(In reply to Jason Orendorff [:jorendorff] from comment #1)

Unfortunately the resulting Error object does not contain any useful stacktrace (because the JavaScript is defined inline). This makes it very hard to locate the error, especially on (legacy) pages with many event handlers defined this way.

Are the filename and line number correct?

Unfortunately they are not. Most HTML pages I have observed the SyntaxError on usually have either the opening <html> or <head> tag on line 2 and the element with the faulty event handler comes much further below.

To be fair: Even if the stack trace lines were accurate, this wouldn't help a lot with pages that are generated dynamically and individually for each user.

Putting some source code into .message is a nice hack, but it also has downsides. It could be confusing; it could break web sites that (unwisely) depend on the text in .message (this really happens). And it's still pretty manual. Something machine-readable would be better, if we can swing it.

Generally, I feel like other JavaScript messages have already been improved in the past (where they hadn't been meaningful enough before) and they are not consistent between browsers anyways. Specifically, I also cannot see why a web site would depend on the missing ) after argument list message (or SyntaxError messages in general), since it is most likely caused by either a typo (during development) or by insufficient escaping. In this case the developers' interest in getting sufficient information to locate the causing code location may weigh more than breaking web sites that theoretically depend on this.

Otherwise, I wouldn't mind setting some magical variable or property to opt-in to "SyntaxError code context enrichment". :)

Comment 5

[Jason Orendorff [:jorendorff]]

There are two hurdles to clear here:

• nailing down the right behavior;
• finding someone with time to do it.

Volunteer effort can help.

I think the SyntaxError object already contains the source line where the error was detected, in errorObject->getErrorReport()->linebuf(). It's certainly possible to expose this somehow. For example, the JS shell already does better than the Firefox DevTools console:

js> Function("n", "retur n")
typein:3:6 SyntaxError: unexpected token: identifier:
typein:3:6 retur n
typein:3:6 ......^

See also JSErrorReport::linebuf_ which is populated in TokenStreamCharsBase::addLineOfContext with the following stack:

  * frame #0: 0x0000000100d9c99f js`js::frontend::TokenStreamCharsBase<char16_t>::addLineOfContext(this=0x00007ffeefbfaa88, err=0x00007ffeefbf93d8, offset=30) at TokenStream.cpp:1738:8
    frame #1: 0x0000000100db0008 js`js::frontend::GeneralTokenStreamChars<char16_t, js::frontend::ParserAnyCharsAccess<js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t> > >::internalComputeLineOfContext(this=0x00007ffeefbfaa88, err=0x00007ffeefbf93d8, offset=30) at TokenStream.h:1978:12
    frame #2: 0x0000000100dc3403 js`js::frontend::TokenStreamSpecific<char16_t, js::frontend::ParserAnyCharsAccess<js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t> > >::computeErrorMetadata(this=0x00007ffeefbfaa80, err=0x00007ffeefbf93d8, errorOffset=0x00007ffeefbf9350) at TokenStream.cpp:1807:12
    frame #3: 0x0000000100c6cf91 js`js::frontend::GeneralParser<js::frontend::FullParseHandler, char16_t>::computeErrorMetadata(this=0x00007ffeefbfa5b0, err=0x00007ffeefbf93d8, offset=0x00007ffeefbf95b8) at Parser.cpp:9144:24
    frame #4: 0x0000000100dd7c21 js`js::frontend::ErrorReportMixin::errorWithNotesAtVA(this=0x00007ffeefbfa5b0, notes=js::UniquePtr<JSErrorNotes> @ 0x00007ffeefbf95b0, offset=0x00007ffeefbf95b8, errorNumber=282, args=0x00007ffeefbf95c0) [1]) at ErrorReporter.h:143:10
    frame #5: 0x0000000100da22ec js`js::frontend::ErrorReportMixin::error(this=0x00007ffeefbfa5b0, errorNumber=282) at ErrorReporter.h:87:5

Comment 6

[Jason Orendorff [:jorendorff]]

IIUC, the root problem is that SyntaxErrors in HTML event handler attributes are unactionable.

Possible remediations (granting that some errors won't be easy to track down no matter what we do):

• fixing the filename and line number
• including the erroneous source on the SyntaxError
• same, but just the source line where the error was detected
• same, but also include it in the .message text

Talking to folks to try to figure out how much of a priority this should be. In particular, what fraction of web developers still care about event handler attributes? (I have to ask because I haven't been a web developer for decades!)