Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug Failure. False expression: JsxText tokens should not be the first child of JsxElement | JsxSelfClosingElement in findRightmostChildNodeWithTokens during getQuickInfoAtPosition #20788

Closed
aozgaa opened this issue Dec 19, 2017 · 11 comments
Assignees
Labels
Bug A bug in TypeScript Needs More Info The issue still hasn't been fully clarified Source: Telemetry The issue relates to the telemetry in editors

Comments

@aozgaa
Copy link
Contributor

aozgaa commented Dec 19, 2017

tsserver version: 2.6.1
hitting sessions: 6483
stack:

Error: Debug Failure. False expression: `JsxText` tokens should not be the first child of `JsxElement | JsxSelfClosingElement`
    at findRightmostChildNodeWithTokens (tsserver.js:62901:30)
    at find (tsserver.js:62893:33)
    at findPrecedingToken (tsserver.js:62859:22)
    at getTokenAtPositionWorker (tsserver.js:62822:41)
    at getTouchingToken (tsserver.js:62794:16)
    at Object.getTouchingPropertyName (tsserver.js:62790:16)
    at Object.getQuickInfoAtPosition (tsserver.js:79791:27)
    at IOSession.Session.getQuickInfoWorker (tsserver.js:86102:62)
    at Session.handlers.ts.createMapFromTemplate._a.(anonymous function) (tsserver.js:85266:61)
    at tsserver.js:86669:88
    at IOSession.Session.executeWithRequestId (tsserver.js:86660:28)
    at IOSession.Session.executeCommand (tsserver.js:86669:33)
    at IOSession.Session.onMessage (tsserver.js:86689:35)
    at Interface.<anonymous> (tsserver.js:87881:27)
    at emitOne (events.js:96:13)
    at Interface.emit (events.js:191:7)
    at Interface._onLine (readline.js:241:10)
    at Interface._normalWrite (readline.js:384:12)
    at Socket.ondata (readline.js:101:10)
    at emitOne (events.js:96:13)
    at Socket.emit (events.js:191:7)
    at readableAddChunk (_stream_readable.js:178:18)
    at Socket.Readable.push (_stream_readable.js:136:10)
    at Pipe.onread (net.js:560:20)
@DanielRosenwasser DanielRosenwasser added the Source: Telemetry The issue relates to the telemetry in editors label Dec 19, 2017
@mhegazy mhegazy assigned ghost Jan 20, 2018
@mhegazy mhegazy added the Bug A bug in TypeScript label Jan 20, 2018
@mhegazy mhegazy added this to the TypeScript 2.8 milestone Jan 20, 2018
@mhegazy
Copy link
Contributor

mhegazy commented Jan 20, 2018

The Source: Telemetry have limited information. we only have the call stack from the crash report. We need to either mitigate the crash, or add additional asserts to get more information in future crash reports.

@ghost
Copy link

ghost commented Jan 22, 2018

The assertion was added in #16385. Since then we've added JSX fragment syntax (#19249), so that might have to do with the error. The first child of some node is a JSXText with containsOnlyWhiteSpaces set --
the error message assumes its parent is JsxElement | JsxSelfClosingElement but it may be a JsxFragment.

@uniqueiniquity Do you think this might have been fixed by #20912?

If it helps, I wrote this helper method in class TestState in fourslash.ts to look for errors:

public verifyPreceding() {
    for (const fileName of this.getProgram().getRootFileNames()) {
        const file = this.getProgram().getSourceFile(fileName);
        for (let i = 0; i < file.text.length; i++) {
            ts.findPrecedingToken(i, file);
        }
    };
}

You also need to add in class Verify: public preceding() { this.state.verifyPreceding(); }

@uniqueiniquity
Copy link
Contributor

@andy-ms I doubt #20912 would have fixed this since it was just related to behavior on format.

@uniqueiniquity
Copy link
Contributor

I am looking into this further.

@uniqueiniquity
Copy link
Contributor

@andy-ms your helper function will not behave the same as the case in the stack trace; the caller of findPrecedingToken provides a third argument.

@uniqueiniquity uniqueiniquity self-assigned this Jan 23, 2018
@uniqueiniquity
Copy link
Contributor

Worked on this with @RyanCavanaugh and could not find a case that would locally repro.
Might be worth adding additional asserts as @mhegazy suggests.

@ghost
Copy link

ghost commented Jan 24, 2018

Maybe add the assert in the parser -- on finishing a jsx node, assert that the first child is not whitespace-only?

@mhegazy
Copy link
Contributor

mhegazy commented Jan 24, 2018

Asserts in the parser would be bad. if they fail nothing will work in the program. at least with this assert, some operations work.

@ghost
Copy link

ghost commented Jan 24, 2018

@mhegazy Can we have informational-only asserts? We don't necessarily need to throw an exception, just report in telemetry that there was an error.

@mhegazy
Copy link
Contributor

mhegazy commented Jan 24, 2018

but we have no way of getting the info really.. we log the failures, but the info can go to the log, and we have no way on getting that afterwards.

@uniqueiniquity
Copy link
Contributor

We've been unable to generate a repro of this, so adding "Needs More Info".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Needs More Info The issue still hasn't been fully clarified Source: Telemetry The issue relates to the telemetry in editors
Projects
None yet
Development

No branches or pull requests

4 participants