Copy paste api should not fail on invalid provider (microsoft#182535)

Author: mjbvz

extensions/vscode-api-tests/src/singlefolder-tests/documentPaste.test.ts

@@ -49,8 +49,8 @@ const textPlain = 'text/plain';
 					const str = await existing.asString();
 					// text/plain includes the trailing new line in this case
 					// On windows, this will always be `\r\n` even if the document uses `\n`
-					const eol = str.match(/\r?\n$/);
-					const reversed = reverseString(str.slice(0, -eol![0].length));
+					const eol = str.match(/\r?\n$/)?.[0] ?? '\n';
+					const reversed = reverseString(str.slice(0, -eol.length));
 					dataTransfer.set(textPlain, new vscode.DataTransferItem(reversed + '\n'));
 				}
 			}
@@ -170,7 +170,32 @@ const textPlain = 'text/plain';
 		const newDocContent = getNextDocumentText(disposables, doc);
 		await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
 		assert.strictEqual(await newDocContent, 'cba\ndef');
+	}));
+
+
+	test('One failing provider should not effect other', usingDisposables(async (disposables) => {
+		const file = await createRandomFile('abc\ndef');
+		const doc = await vscode.workspace.openTextDocument(file);
+
+		const editor = await vscode.window.showTextDocument(doc);
+		editor.selections = [new vscode.Selection(0, 0, 0, 3)];
+
+		disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
+			async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
+				dataTransfer.set(textPlain, new vscode.DataTransferItem('xyz'));
+			}
+		}, { copyMimeTypes: [textPlain] }));
 
+		disposables.push(vscode.languages.registerDocumentPasteEditProvider({ language: 'plaintext' }, new class implements vscode.DocumentPasteEditProvider {
+			async prepareDocumentPaste(_document: vscode.TextDocument, _ranges: readonly vscode.Range[], _dataTransfer: vscode.DataTransfer, _token: vscode.CancellationToken): Promise<void> {
+				throw new Error('Expected testing error from bad provider');
+			}
+		}, { copyMimeTypes: [textPlain] }));
+
+		await vscode.commands.executeCommand('editor.action.clipboardCopyAction');
+		const newDocContent = getNextDocumentText(disposables, doc);
+		await vscode.commands.executeCommand('editor.action.clipboardPasteAction');
+		assert.strictEqual(await newDocContent, 'xyz\ndef');
 	}));
 });
 
@@ -181,10 +206,9 @@ function reverseString(str: string) {
 function getNextDocumentText(disposables: vscode.Disposable[], doc: vscode.TextDocument): Promise<string> {
 	return new Promise<string>(resolve => {
 		disposables.push(vscode.workspace.onDidChangeTextDocument(e => {
-			if (e.document === doc) {
-				resolve(doc.getText());
+			if (e.document.uri.fsPath === doc.uri.fsPath) {
+				resolve(e.document.getText());
 			}
 		}));
 	});
 }
-

src/vs/editor/contrib/dropOrPasteInto/browser/copyPasteController.ts

@@ -165,8 +165,13 @@ export class CopyPasteController extends Disposable implements IEditorContributi
 		});
 
 		const promise = createCancelablePromise(async token => {
-			const results = coalesce(await Promise.all(providers.map(provider => {
-				return provider.prepareDocumentPaste!(model, ranges, dataTransfer, token);
+			const results = coalesce(await Promise.all(providers.map(async provider => {
+				try {
+					return await provider.prepareDocumentPaste!(model, ranges, dataTransfer, token);
+				} catch (err) {
+					console.error(err);
+					return undefined;
+				}
 			})));
 
 			// Values from higher priority providers should overwrite values from lower priority ones.
@@ -391,9 +396,14 @@ export class CopyPasteController extends Disposable implements IEditorContributi
 
 	private async getPasteEdits(providers: readonly DocumentPasteEditProvider[], dataTransfer: VSDataTransfer, model: ITextModel, selections: readonly Selection[], token: CancellationToken): Promise<DocumentPasteEdit[]> {
 		const result = await raceCancellation(
-			Promise.all(
-				providers.map(provider => provider.provideDocumentPasteEdits?.(model, selections, dataTransfer, token))
-			).then(coalesce),
+			Promise.all(providers.map(provider => {
+				try {
+					return provider.provideDocumentPasteEdits?.(model, selections, dataTransfer, token);
+				} catch (err) {
+					console.error(err);
+					return undefined;
+				}
+			})).then(coalesce),
 			token);
 		result?.sort((a, b) => b.priority - a.priority);
 		return result ?? [];