Skip to content

Commit

Permalink
Don't treat cancellation as an error
Browse files Browse the repository at this point in the history 
**Problem**
In the ts server communication, canceling a request currently throws an exception. This is wrong because  cancellation is not an error. It also means that we need to wrap every server call in a generic try catch to handle cancellation. There are no checks in place to distinquish a cancellation from a rea
  • Loading branch information
@mjbvz
mjbvz committed Sep 20, 2018
1 parent df411e9 commit 97c753f
Show file tree
Hide file tree
Showing 22 changed files with 235 additions and 283 deletions.
32 changes: 14 additions & 18 deletions extensions/typescript-language-features/src/features/baseCodeLensProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import * as vscode from 'vscode';
import * as Proto from '../protocol';
import { ITypeScriptServiceClient } from '../typescriptService';
import { ITypeScriptServiceClient, ServerResponse } from '../typescriptService';
import { escapeRegExp } from '../utils/regexp';
import * as typeConverters from '../utils/typeConverters';

Expand All @@ -21,13 +21,13 @@ export class ReferencesCodeLens extends vscode.CodeLens {
}

export class CachedNavTreeResponse {
private response?: Promise<Proto.NavTreeResponse>;
private response?: Promise<ServerResponse<Proto.NavTreeResponse>>;
private version: number = -1;
private document: string = '';

public execute(
document: vscode.TextDocument,
f: () => Promise<Proto.NavTreeResponse>
f: () => Promise<ServerResponse<Proto.NavTreeResponse>>
) {
if (this.matches(document)) {
return this.response;
Expand All @@ -42,8 +42,8 @@ export class CachedNavTreeResponse {

private update(
document: vscode.TextDocument,
response: Promise<Proto.NavTreeResponse>
): Promise<Proto.NavTreeResponse> {
response: Promise<ServerResponse<Proto.NavTreeResponse>>
): Promise<ServerResponse<Proto.NavTreeResponse>> {
this.response = response;
this.version = document.version;
this.document = document.uri.toString();
Expand All @@ -69,21 +69,17 @@ export abstract class TypeScriptBaseCodeLensProvider implements vscode.CodeLensP
return [];
}

try {
const response = await this.cachedResponse.execute(document, () => this.client.execute('navtree', { file: filepath }, token));
if (!response) {
return [];
}

const tree = response.body;
const referenceableSpans: vscode.Range[] = [];
if (tree && tree.childItems) {
tree.childItems.forEach(item => this.walkNavTree(document, item, null, referenceableSpans));
}
return referenceableSpans.map(span => new ReferencesCodeLens(document.uri, filepath, span));
} catch {
const response = await this.cachedResponse.execute(document, () => this.client.execute('navtree', { file: filepath }, token));
if (!response || response.type !== 'response') {
return [];
}

const tree = response.body;
const referenceableSpans: vscode.Range[] = [];
if (tree && tree.childItems) {
tree.childItems.forEach(item => this.walkNavTree(document, item, null, referenceableSpans));
}
return referenceableSpans.map(span => new ReferencesCodeLens(document.uri, filepath, span));
}

protected abstract extractSymbol(
Expand Down
45 changes: 24 additions & 21 deletions extensions/typescript-language-features/src/features/completions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,23 +321,20 @@ class TypeScriptCompletionItemProvider implements vscode.CompletionItemProvider

let isNewIdentifierLocation = true;
let msg: ReadonlyArray<Proto.CompletionEntry> | undefined = undefined;
try {
if (this.client.apiVersion.gte(API.v300)) {
const { body } = await this.client.interuptGetErr(() => this.client.execute('completionInfo', args, token));
if (!body) {
return null;
}
isNewIdentifierLocation = body.isNewIdentifierLocation;
msg = body.entries;
} else {
const { body } = await this.client.interuptGetErr(() => this.client.execute('completions', args, token));
if (!body) {
return null;
}
msg = body;
if (this.client.apiVersion.gte(API.v300)) {
const response = await this.client.interuptGetErr(() => this.client.execute('completionInfo', args, token));
if (response.type !== 'response' || !response.body) {
return null;
}
} catch {
return null;
isNewIdentifierLocation = response.body.isNewIdentifierLocation;
msg = response.body.entries;
} else {
const response = await this.client.interuptGetErr(() => this.client.execute('completions', args, token));
if (response.type !== 'response' || !response.body) {
return null;
}

msg = response.body;
}

const isInValidCommitCharacterContext = this.isInValidCommitCharacterContext(document, position);
Expand Down Expand Up @@ -371,12 +368,12 @@ class TypeScriptCompletionItemProvider implements vscode.CompletionItemProvider
};

let details: Proto.CompletionEntryDetails[] | undefined;
try {
const { body } = await this.client.execute('completionEntryDetails', args, token);
details = body;
} catch {
const response = await this.client.execute('completionEntryDetails', args, token);
if (response.type !== 'response') {
return item;
}
const { body } = response;
details = body;

if (!details || !details.length || !details[0]) {
return item;
Expand Down Expand Up @@ -528,7 +525,13 @@ class TypeScriptCompletionItemProvider implements vscode.CompletionItemProvider
// Workaround for https://github.com/Microsoft/TypeScript/issues/12677
// Don't complete function calls inside of destructive assigments or imports
try {
const { body } = await this.client.execute('quickinfo', typeConverters.Position.toFileLocationRequestArgs(filepath, position), token);
const args: Proto.FileLocationRequestArgs = typeConverters.Position.toFileLocationRequestArgs(filepath, position);
const response = await this.client.execute('quickinfo', args, token);
if (response.type !== 'response') {
return true;
}

const { body } = response;
switch (body && body.kind) {
case 'var':
case 'let':
Expand Down
13 changes: 7 additions & 6 deletions extensions/typescript-language-features/src/features/definitionProviderBase.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,14 @@ export default class TypeScriptDefinitionProviderBase {
}

const args = typeConverters.Position.toFileLocationRequestArgs(filepath, position);
try {
const response = await this.client.execute(definitionType, args, token);
const locations: Proto.FileSpan[] = (response && response.body) || [];
return locations.map(location =>
typeConverters.Location.fromTextSpan(this.client.toResource(location.file), location));
} catch {

const response = await this.client.execute(definitionType, args, token);
if (response.type !== 'response') {
return undefined;
}

const locations: Proto.FileSpan[] = (response && response.body) || [];
return locations.map(location =>
typeConverters.Location.fromTextSpan(this.client.toResource(location.file), location));
}
}
32 changes: 14 additions & 18 deletions extensions/typescript-language-features/src/features/definitions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,21 @@ export default class TypeScriptDefinitionProvider extends DefinitionProviderBase
}

const args = typeConverters.Position.toFileLocationRequestArgs(filepath, position);
try {
const { body } = await this.client.execute('definitionAndBoundSpan', args, token);
if (!body) {
return undefined;
}

const span = body.textSpan ? typeConverters.Range.fromTextSpan(body.textSpan) : undefined;
return body.definitions
.map(location => {
const target = typeConverters.Location.fromTextSpan(this.client.toResource(location.file), location);
return <vscode.DefinitionLink>{
originSelectionRange: span,
targetRange: target.range,
targetUri: target.uri,
};
});
} catch {
return [];
const response = await this.client.execute('definitionAndBoundSpan', args, token);
if (response.type !== 'response' || !response.body) {
return undefined;
}

const span = response.body.textSpan ? typeConverters.Range.fromTextSpan(response.body.textSpan) : undefined;
return response.body.definitions
.map(location => {
const target = typeConverters.Location.fromTextSpan(this.client.toResource(location.file), location);
return <vscode.DefinitionLink>{
originSelectionRange: span,
targetRange: target.range,
targetUri: target.uri,
};
});
}

return this.getSymbolLocations('definition', document, position, token);
Expand Down
12 changes: 3 additions & 9 deletions extensions/typescript-language-features/src/features/documentHighlight.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,12 @@ class TypeScriptDocumentHighlightProvider implements vscode.DocumentHighlightPro
}

const args = typeConverters.Position.toFileLocationRequestArgs(file, position);
let items: Proto.OccurrencesResponseItem[];
try {
const { body } = await this.client.execute('occurrences', args, token);
if (!body) {
return [];
}
items = body;
} catch {
const response = await this.client.execute('occurrences', args, token);
if (response.type !== 'response' || !response.body) {
return [];
}

return items
return response.body
.filter(x => !x.isInString)
.map(documentHighlightFromOccurance);
}
Expand Down
13 changes: 4 additions & 9 deletions extensions/typescript-language-features/src/features/documentSymbol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,13 @@ class TypeScriptDocumentSymbolProvider implements vscode.DocumentSymbolProvider
return undefined;
}

let tree: Proto.NavigationTree;
try {
const args: Proto.FileRequestArgs = { file };
const { body } = await this.client.execute('navtree', args, token);
if (!body) {
return undefined;
}
tree = body;
} catch {
const args: Proto.FileRequestArgs = { file };
const response = await this.client.execute('navtree', args, token);
if (response.type !== 'response' || !response.body) {
return undefined;
}

let tree = response.body;
if (tree && tree.childItems) {
// The root represents the file. Ignore this when showing in the UI
const result: vscode.DocumentSymbol[] = [];
Expand Down
12 changes: 3 additions & 9 deletions extensions/typescript-language-features/src/features/folding.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,12 @@ class TypeScriptFoldingProvider implements vscode.FoldingRangeProvider {
}

const args: Proto.FileRequestArgs = { file };
let body: Proto.OutliningSpan[] | undefined;
try {
body = (await this.client.execute('getOutliningSpans', args, token)).body;
} catch {
// noop
}

if (!body) {
const response = await this.client.execute('getOutliningSpans', args, token);
if (response.type !== 'response' || !response.body) {
return;
}

return body
return response.body
.map(span => this.convertOutliningSpan(span, document))
.filter(foldingRange => !!foldingRange) as vscode.FoldingRange[];
}
Expand Down
62 changes: 27 additions & 35 deletions extensions/typescript-language-features/src/features/formatting.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,13 @@ class TypeScriptFormattingProvider implements vscode.DocumentRangeFormattingEdit

await this.formattingOptionsManager.ensureConfigurationOptions(document, options, token);

let edits: Proto.CodeEdit[];
try {
const args = typeConverters.Range.toFormattingRequestArgs(file, range);
const { body } = await this.client.execute('format', args, token);
if (!body) {
return undefined;
}
edits = body;
} catch {
const args = typeConverters.Range.toFormattingRequestArgs(file, range);
const response = await this.client.execute('format', args, token);
if (response.type !== 'response' || !response.body) {
return undefined;
}

return edits.map(typeConverters.TextEdit.fromCodeEdit);
return response.body.map(typeConverters.TextEdit.fromCodeEdit);
}

public async provideOnTypeFormattingEdits(
Expand All @@ -62,35 +56,33 @@ class TypeScriptFormattingProvider implements vscode.DocumentRangeFormattingEdit
...typeConverters.Position.toFileLocationRequestArgs(file, position),
key: ch
};
try {
const { body } = await this.client.execute('formatonkey', args, token);
const edits = body;
const result: vscode.TextEdit[] = [];
if (!edits) {
return result;
}
for (const edit of edits) {
const textEdit = typeConverters.TextEdit.fromCodeEdit(edit);
const range = textEdit.range;
// Work around for https://github.com/Microsoft/TypeScript/issues/6700.
// Check if we have an edit at the beginning of the line which only removes white spaces and leaves
// an empty line. Drop those edits
if (range.start.character === 0 && range.start.line === range.end.line && textEdit.newText === '') {
const lText = document.lineAt(range.start.line).text;
// If the edit leaves something on the line keep the edit (note that the end character is exclusive).
// Keep it also if it removes something else than whitespace
if (lText.trim().length > 0 || lText.length > range.end.character) {
result.push(textEdit);
}
} else {
const response = await this.client.execute('formatonkey', args, token);
if (response.type !== 'response' || !response.body) {
return [];
}
const edits = response.body;
const result: vscode.TextEdit[] = [];
if (!edits) {
return result;
}
for (const edit of edits) {
const textEdit = typeConverters.TextEdit.fromCodeEdit(edit);
const range = textEdit.range;
// Work around for https://github.com/Microsoft/TypeScript/issues/6700.
// Check if we have an edit at the beginning of the line which only removes white spaces and leaves
// an empty line. Drop those edits
if (range.start.character === 0 && range.start.line === range.end.line && textEdit.newText === '') {
const lText = document.lineAt(range.start.line).text;
// If the edit leaves something on the line keep the edit (note that the end character is exclusive).
// Keep it also if it removes something else than whitespace
if (lText.trim().length > 0 || lText.length > range.end.character) {
result.push(textEdit);
}
} else {
result.push(textEdit);
}
return result;
} catch {
// noop
}
return [];
return result;
}
}

Expand Down
17 changes: 7 additions & 10 deletions extensions/typescript-language-features/src/features/hover.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,14 @@ class TypeScriptHoverProvider implements vscode.HoverProvider {
}

const args = typeConverters.Position.toFileLocationRequestArgs(filepath, position);
try {
const { body } = await this.client.interuptGetErr(() => this.client.execute('quickinfo', args, token));
if (body) {
return new vscode.Hover(
TypeScriptHoverProvider.getContents(body),
typeConverters.Range.fromTextSpan(body));
}
} catch (e) {
// noop
const response = await this.client.interuptGetErr(() => this.client.execute('quickinfo', args, token));
if (response.type !== 'response' || !response.body) {
return undefined;
}
return undefined;

return new vscode.Hover(
TypeScriptHoverProvider.getContents(response.body),
typeConverters.Range.fromTextSpan(response.body));
}

private static getContents(
Expand Down
Loading

0 comments on commit 97c753f

Please sign in to comment.