Skip to content

Commit

Permalink
Handling of unhandled Promise rejections (#10333)
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne authored Jun 8, 2022
1 parent 836e18a commit fb7cd13
Show file tree
Hide file tree
Showing 80 changed files with 566 additions and 408 deletions.
7 changes: 6 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,12 @@ module.exports = {
'no-useless-constructor': 'off',
'@typescript-eslint/no-useless-constructor': 'error',
'@typescript-eslint/no-var-requires': 'off',
'@typescript-eslint/no-floating-promises': 'error',
'@typescript-eslint/no-floating-promises': [
'error',
{
ignoreVoid: false
}
],

// Other rules
'class-methods-use-this': 'off',
Expand Down
6 changes: 6 additions & 0 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,9 @@ jobs:
id: test_unittests
run: npm run test:unittests

- name: Verify there are no unhandled errors
run: npm run verifyUnhandledErrors

- name: Publish Test Report
uses: scacap/action-surefire-report@v1
if: steps.test_unittests.outcome == 'failure' && failure()
Expand Down Expand Up @@ -728,6 +731,9 @@ jobs:
path: './SD-memtest.json'
retention-days: 20

- name: Verify there are no unhandled errors
run: npm run verifyUnhandledErrors

smoke-tests:
timeout-minutes: 30
name: Smoke tests
Expand Down
15 changes: 13 additions & 2 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -464,9 +464,20 @@ gulp.task('validateDependencies', async () => {
console.log(modules);
if (modules.length > 1 || modules[0] !== '@jupyterlab/coreutils') {
// we already validate that we are not using moment in @jupyterlab/coreutils
const message = `The following modules require moment: ${modules.join(', ')}. Please validate if moment is being used.`;
const message = `The following modules require moment: ${modules.join(
', '
)}. Please validate if moment is being used.`;
console.error(message);
throw new Error(message);
}
}
});
});

gulp.task('verifyUnhandledErrors', async () => {
const fileName = path.join(__dirname, 'unhandledErrors.txt');
const contents = fs.pathExistsSync(fileName) ? fs.readFileSync(fileName, 'utf8') : '';
if (contents.trim().length) {
console.error(contents);
throw new Error('Unhandled errors detected. Please fix them before merging this PR.', contents);
}
});
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2125,6 +2125,7 @@
"startJupyterServer": "node build/preDebugWebTest.js",
"stopJupyterServer": "node build/postDebugWebTest.js",
"validateTelemetryMD": "gulp validateTelemetryMD",
"verifyUnhandledErrors": "gulp verifyUnhandledErrors",
"prepare": "husky install"
},
"dependencies": {
Expand Down
6 changes: 3 additions & 3 deletions src/extension.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ async function handleError(ex: Error, startupDurations: Record<string, number>)

function notifyUser(msg: string) {
try {
void window.showErrorMessage(msg);
window.showErrorMessage(msg).then(noop, noop);
} catch (ex) {
traceError('failed to notify user', ex);
}
Expand Down Expand Up @@ -300,9 +300,9 @@ async function activateLegacy(
});
serviceManager.addSingletonInstance<Promise<boolean>>(IsPreRelease, isPreReleasePromise);
if (isDevMode) {
void commands.executeCommand('setContext', 'jupyter.development', true);
commands.executeCommand('setContext', 'jupyter.development', true).then(noop, noop);
}
void commands.executeCommand('setContext', 'jupyter.webExtension', false);
commands.executeCommand('setContext', 'jupyter.webExtension', false).then(noop, noop);

// Set the logger home dir (we can compute this in a node app)
setHomeDirectory(require('untildify')('~') || '');
Expand Down
6 changes: 3 additions & 3 deletions src/extension.web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ async function handleError(ex: Error, startupDurations: Record<string, number>)

function notifyUser(msg: string) {
try {
void window.showErrorMessage(msg);
window.showErrorMessage(msg).then(noop, noop);
} catch (ex) {
traceError('failed to notify user', ex);
}
Expand Down Expand Up @@ -274,9 +274,9 @@ async function activateLegacy(
serviceManager.addSingletonInstance<boolean>(IsDevMode, isDevMode);
serviceManager.addSingletonInstance<boolean>(IsWebExtension, true);
if (isDevMode) {
void commands.executeCommand('setContext', 'jupyter.development', true);
commands.executeCommand('setContext', 'jupyter.development', true).then(noop, noop);
}
void commands.executeCommand('setContext', 'jupyter.webExtension', true);
commands.executeCommand('setContext', 'jupyter.webExtension', true).then(noop, noop);

// Output channel is special. We need it before everything else
addOutputChannel(context, serviceManager, isDevMode);
Expand Down
17 changes: 10 additions & 7 deletions src/intellisense/logReplayService.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { IFileSystemNode } from '../platform/common/platform/types.node';
import { IDisposableRegistry, IConfigurationService } from '../platform/common/types';
import { Commands, EditorContexts } from '../webviews/webview-side/common/constants';
import { sleep, waitForCondition } from '../platform/common/utils/async';
import { noop, swallowExceptions } from '../platform/common/utils/misc';

/**
* Class used to replay pylance log output to regenerate a series of edits.
Expand Down Expand Up @@ -55,7 +56,7 @@ export class LogReplayService implements IExtensionSingleActivationService {
this.commandService.registerCommand(Commands.ReplayPylanceLogStep, this.step, this)
);
this.isLogActive = new ContextKey(EditorContexts.ReplayLogLoaded, this.commandService);
void this.isLogActive.set(false);
this.isLogActive.set(false).then(noop, noop);
}

private async replayPylanceLog() {
Expand All @@ -68,7 +69,7 @@ export class LogReplayService implements IExtensionSingleActivationService {
void this.isLogActive?.set(true);
}
} else {
void this.appShell.showErrorMessage(`Command should be run with a jupyter notebook open`);
this.appShell.showErrorMessage(`Command should be run with a jupyter notebook open`).then(noop, noop);
}
}

Expand All @@ -79,7 +80,9 @@ export class LogReplayService implements IExtensionSingleActivationService {
this.activeNotebook === vscode.window.activeNotebookEditor?.notebook &&
this.activeNotebook
) {
void this.appShell.showInformationMessage(`Replaying step ${this.index + 2} of ${this.steps.length}`);
this.appShell
.showInformationMessage(`Replaying step ${this.index + 2} of ${this.steps.length}`)
.then(noop, noop);

// Move to next step
this.index += 1;
Expand Down Expand Up @@ -220,17 +223,17 @@ export class LogReplayService implements IExtensionSingleActivationService {
});
}
if (this.steps.length === this.index) {
void this.isLogActive?.set(false);
swallowExceptions(() => this.isLogActive?.set(false));
this.steps = [];
this.index = -1;
}
} else if (
this.activeNotebook?.toString() !== vscode.window.activeNotebookEditor?.notebook.uri.toString() &&
this.index < this.steps.length - 1
) {
void this.appShell.showErrorMessage(
`You changed the notebook editor in the middle of stepping through the log`
);
this.appShell
.showErrorMessage(`You changed the notebook editor in the middle of stepping through the log`)
.then(noop, noop);
}
}

Expand Down
15 changes: 9 additions & 6 deletions src/interactive-window/commands/commandRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ export class CommandRegistry implements IDisposable, IExtensionSingleActivationS
this.registerCommand(Commands.RunCurrentCell, this.runCurrentCell);
this.registerCommand(Commands.RunCurrentCellAdvance, this.runCurrentCellAndAdvance);
this.registerCommand(Commands.ExecSelectionInInteractiveWindow, (textOrUri: string | undefined | Uri) => {
void this.runSelectionOrLine(textOrUri);
this.runSelectionOrLine(textOrUri).catch(noop);
});
this.registerCommand(Commands.RunAllCellsAbove, this.runAllCellsAbove);
this.registerCommand(Commands.RunCellAndAllBelow, this.runCellAndAllBelow);
Expand Down Expand Up @@ -334,7 +334,7 @@ export class CommandRegistry implements IDisposable, IExtensionSingleActivationS
private async debugStepOver(): Promise<void> {
// Make sure that we are in debug mode
if (this.debugService?.activeDebugSession) {
void this.commandManager.executeCommand('workbench.action.debug.stepOver');
this.commandManager.executeCommand('workbench.action.debug.stepOver').then(noop, noop);
}
}

Expand All @@ -352,15 +352,15 @@ export class CommandRegistry implements IDisposable, IExtensionSingleActivationS
}
}

void this.commandManager.executeCommand('workbench.action.debug.stop');
this.commandManager.executeCommand('workbench.action.debug.stop').then(noop, noop);
}
}

@captureTelemetry(Telemetry.DebugContinue)
private async debugContinue(): Promise<void> {
// Make sure that we are in debug mode
if (this.debugService?.activeDebugSession) {
void this.commandManager.executeCommand('workbench.action.debug.continue');
this.commandManager.executeCommand('workbench.action.debug.continue').then(noop, noop);
}
}

Expand Down Expand Up @@ -508,7 +508,10 @@ export class CommandRegistry implements IDisposable, IExtensionSingleActivationS
}

private openPythonExtensionPage() {
void env.openExternal(Uri.parse(`https://marketplace.visualstudio.com/items?itemName=ms-toolsai.jupyter`));
env.openExternal(Uri.parse(`https://marketplace.visualstudio.com/items?itemName=ms-toolsai.jupyter`)).then(
noop,
noop
);
}

// Open up our variable viewer using the command that VS Code provides for this
Expand Down Expand Up @@ -576,7 +579,7 @@ export class CommandRegistry implements IDisposable, IExtensionSingleActivationS
} catch (e) {
sendTelemetryEvent(EventName.OPEN_DATAVIEWER_FROM_VARIABLE_WINDOW_ERROR, undefined, undefined, e);
traceError(e);
void this.errorHandler.handleError(e);
this.errorHandler.handleError(e).then(noop, noop);
}
}
}
Expand Down
24 changes: 10 additions & 14 deletions src/interactive-window/commands/exportCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { IApplicationShell, ICommandManager, IVSCodeNotebook } from '../../platf
import { traceInfo } from '../../platform/logging';
import { IDisposable } from '../../platform/common/types';
import { DataScience } from '../../platform/common/utils/localize';
import { isUri } from '../../platform/common/utils/misc';
import { isUri, noop } from '../../platform/common/utils/misc';
import { PythonEnvironment } from '../../platform/pythonEnvironments/info';
import { sendTelemetryEvent } from '../../telemetry';
import { INotebookControllerManager } from '../../notebooks/types';
Expand Down Expand Up @@ -141,7 +141,9 @@ export class ExportCommands implements IExportCommands, IDisposable {
sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, {
format: ExportFormat.python
});
void this.commandManager.executeCommand(Commands.ExportAsPythonScript, sourceDocument, interpreter);
this.commandManager
.executeCommand(Commands.ExportAsPythonScript, sourceDocument, interpreter)
.then(noop, noop);
}
});
}
Expand All @@ -155,12 +157,9 @@ export class ExportCommands implements IExportCommands, IDisposable {
sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, {
format: ExportFormat.html
});
void this.commandManager.executeCommand(
Commands.ExportToHTML,
sourceDocument,
defaultFileName,
interpreter
);
this.commandManager
.executeCommand(Commands.ExportToHTML, sourceDocument, defaultFileName, interpreter)
.then(noop, noop);
}
},
{
Expand All @@ -170,12 +169,9 @@ export class ExportCommands implements IExportCommands, IDisposable {
sendTelemetryEvent(Telemetry.ClickedExportNotebookAsQuickPick, undefined, {
format: ExportFormat.pdf
});
void this.commandManager.executeCommand(
Commands.ExportToPDF,
sourceDocument,
defaultFileName,
interpreter
);
this.commandManager
.executeCommand(Commands.ExportToPDF, sourceDocument, defaultFileName, interpreter)
.then(noop, noop);
}
}
]
Expand Down
5 changes: 3 additions & 2 deletions src/interactive-window/debugger/jupyter/debuggingManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { DebuggingManagerBase } from '../../../notebooks/debugger/debuggingManag
import { IConfigurationService } from '../../../platform/common/types';
import { IFileGeneratedCodes } from '../../editor-integration/types';
import { buildSourceMap } from '../helper';
import { noop } from '../../../platform/common/utils/misc';

/**
* The DebuggingManager maintains the mapping between notebook documents and debug sessions.
Expand Down Expand Up @@ -91,7 +92,7 @@ export class InteractiveWindowDebuggingManager
return;
case IpykernelCheckResult.Outdated:
case IpykernelCheckResult.Unknown: {
void this.promptInstallIpykernel6();
this.promptInstallIpykernel6().then(noop, noop);
return;
}
case IpykernelCheckResult.Ok: {
Expand Down Expand Up @@ -157,7 +158,7 @@ export class InteractiveWindowDebuggingManager
return;
}
if (!kernel?.session) {
void this.appShell.showInformationMessage(DataScience.kernelWasNotStarted());
this.appShell.showInformationMessage(DataScience.kernelWasNotStarted()).then(noop, noop);
return;
}
const adapter = new KernelDebugAdapter(
Expand Down
Loading

0 comments on commit fb7cd13

Please sign in to comment.