Error: Debug Failure. False expression: ScriptVersionCache should not be set in `getLineMap` during `getApplicableRefactors`

[aozgaa]

tsserver version: 2.6.1
hitting sessions: 1258
stack:

Error: Debug Failure. False expression: ScriptVersionCache should not be set
    at TextStorage.getLineMap (tsserver.js:81599:26)
    at TextStorage.lineOffsetToPosition (tsserver.js:81564:70)
    at ScriptInfo.lineOffsetToPosition (tsserver.js:81784:41)
    at IOSession.Session.getStartAndEndPosition (tsserver.js:86542:48)
    at IOSession.Session.extractPositionAndRange (tsserver.js:86462:35)
    at IOSession.Session.getApplicableRefactors (tsserver.js:86473:31)
    at Session.handlers.ts.createMapFromTemplate._a.(anonymous function) (tsserver.js:85436: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)

[mhegazy]

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.

[amcasey]

@Andy-MS, @sheetalkamat Are you familiar with the purpose of that map?

@amcasey The map is so we can convert a (line, column) position to an index into the text. We want to cache the map since it takes time to compute, but we have to invalidate that cache if the text ever changes.

The error is because in lineOffsetToPosition we do:

if (!this.useScriptVersionCacheIfValidOrOpen()) {
    ...this.getLineMap()...
}

In this.getLineMap we assert that !this.svc. But in useScriptVersionCacheIfValidOrOpen, we may return false if this.pendingReloadFromDisk, even if this.svc is set.

@sheetalkamat Can we just set this.svc = undefined; in delayReloadFromFileIntoText? Then we don't have to check for that in useScriptVersionCacheIfValidOrOpen.

[sheetalkamat]

@Andy-MS no we shouldnt set svc to undefined because we can use the same svc if the text hasnt changed.. if file is opened and closed which many times happens during file/symbol navigations.. This helps in optimizing... Let me look into this tomorrow when i am in office to see whats going on..

[amcasey]

@sheetalkamat Did you have a chance to look into this?

[amcasey]

Breakdown by command over the last five days:

command	count
quickinfo	27645
getApplicableRefactors	13515
occurrences	12603
completions	12034
signatureHelp	2354
definition	2256
completionEntryDetails	413
format	291
getCodeFixes	68
navto	11
formatonkey	10
references	9
rename	1