Merge pull request #8251 from blink1073/handle-session-error-1.x

[1.2.x] Fix session context error handling

docs/source/getting_started/changelog.rst

@@ -7,38 +7,38 @@ JupyterLab Changelog
 ---------------------------------------------------------------------------
 
 v1.2.11
-~~~~~~~
+^^^^^^^
 * Expose global app instance with command-line option: (`#8218 <https://github.com/jupyterlab/jupyterlab/pull/8218>`__)
 
 v1.2.10
-~~~~~~~
+^^^^^^^
 * Fix cwd for kernels (`#8212 <https://github.com/jupyterlab/jupyterlab/pull/8212>`__)
 
 v1.2.9
-~~~~~~~
+^^^^^^^
 * Better handling of slow kernel startup (`#8189 <https://github.com/jupyterlab/jupyterlab/pull/8189>`__)
 * Clean up kernel startup error messages (`#8186 <https://github.com/jupyterlab/jupyterlab/pull/8186>`__)
 
 v1.2.8
-~~~~~~~
+^^^^^^^
 * Fix Widget Tracker behavior (`#8118 <https://github.com/jupyterlab/jupyterlab/pull/8118>`__)
 * Increase timeout for yarn (`#8108 <https://github.com/jupyterlab/jupyterlab/pull/8108>`__)
 * Add .yarnrc explicitly in Python setup (`#8107 <https://github.com/jupyterlab/jupyterlab/pull/8107>`__)
 
 v1.2.7
-~~~~~~~
+^^^^^^^
 * Better handling of fallback versions for an extension (`#7981 <https://github.com/jupyterlab/jupyterlab/pull/7981>`__)
 
 v1.2.6
-~~~~~~~
+^^^^^^^
 * Fix browser check (`#7817 <https://github.com/jupyterlab/jupyterlab/pull/7817>`__)
 
 
 `v1.2.0 <https://github.com/jupyterlab/jupyterlab/releases/tag/v1.2.0>`__
 ---------------------------------------------------------------------------
 
 October 29, 2019
-~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^
 
 Here are some highlights for this release. See the `JupyterLab
 1.2.0 <https://github.com/jupyterlab/jupyterlab/milestone/38?closed=1>`__

examples/example_check.py

@@ -57,7 +57,7 @@ def run_browser(url):
     if not osp.exists(osp.join(target, 'node_modules')):
         os.makedirs(target)
         subprocess.call(["jlpm", "init", "-y"], cwd=target)
-        subprocess.call(["jlpm", "add", "puppeteer"], cwd=target)
+        subprocess.call(["jlpm", "add", "puppeteer@^2"], cwd=target)
     shutil.copy(osp.join(here, 'chrome-example-test.js'), osp.join(target, 'chrome-example-test.js'))
     return subprocess.check_call(["node", "chrome-example-test.js", url], cwd=target)
 

jupyterlab/browser_check.py

@@ -132,7 +132,7 @@ def run_browser(url):
     if not osp.exists(osp.join(target, 'node_modules')):
         os.makedirs(target)
         subprocess.call(["jlpm", "init", "-y"], cwd=target)
-        subprocess.call(["jlpm", "add", "puppeteer"], cwd=target)
+        subprocess.call(["jlpm", "add", "puppeteer@^2"], cwd=target)
     shutil.copy(osp.join(here, 'chrome-test.js'), osp.join(target, 'chrome-test.js'))
     return subprocess.check_call(["node", "chrome-test.js", url], cwd=target)
 

lint-staged.config.js

@@ -0,0 +1,35 @@
+const escape = require('shell-quote').quote;
+const fs = require('fs');
+const isWin = process.platform === 'win32';
+
+const escapeFileNames = filenames =>
+  filenames
+    .filter(filename => fs.existsSync(filename))
+    .map(filename => `"${isWin ? filename : escape([filename])}"`)
+    .join(' ');
+
+module.exports = {
+  '**/*{.css,.json,.md}': filenames => {
+    const escapedFileNames = escapeFileNames(filenames);
+    return [
+      `prettier --write ${escapedFileNames}`,
+      `git add ${escapedFileNames}`
+    ];
+  },
+  '**/*{.ts,.tsx}': filenames => {
+    const escapedFileNames = escapeFileNames(filenames);
+    return [
+      `prettier --write ${escapedFileNames}`,
+      `tslint --fix ${escapedFileNames}`,
+      `git add ${escapedFileNames}`
+    ];
+  },
+  '**/*{.js,.jsx}': filenames => {
+    const escapedFileNames = escapeFileNames(filenames);
+    return [
+      `prettier --write ${escapedFileNames}`,
+      `eslint --fix ${escapedFileNames}`,
+      `git add ${escapedFileNames}`
+    ];
+  }
+};

package.json

@@ -98,8 +98,9 @@
     "eslint-plugin-prettier": "^3.0.1",
     "husky": "^2.3.0",
     "lerna": "^3.13.2",
-    "lint-staged": "^8.1.5",
+    "lint-staged": "^9.5.0",
     "prettier": "^1.18.2",
+    "shell-quote": "^1.7.2",
     "tslint": "^5.15.0",
     "tslint-config-prettier": "^1.18.0",
     "tslint-plugin-prettier": "^2.0.1",

packages/apputils/src/clientsession.tsx

@@ -733,33 +733,40 @@ export class ClientSession implements IClientSession {
       this._session.dispose();
     }
     this._session = session;
-    if (session.path !== this._path) {
-      this._path = session.path;
-      this._propertyChanged.emit('path');
-    }
-    if (session.name !== this._name) {
-      this._name = session.name;
-      this._propertyChanged.emit('name');
-    }
-    if (session.type !== this._type) {
-      this._type = session.type;
-      this._propertyChanged.emit('type');
-    }
-
-    session.terminated.connect(this._onTerminated, this);
-    session.propertyChanged.connect(this._onPropertyChanged, this);
-    session.kernelChanged.connect(this._onKernelChanged, this);
-    session.statusChanged.connect(this._onStatusChanged, this);
-    session.iopubMessage.connect(this._onIopubMessage, this);
-    session.unhandledMessage.connect(this._onUnhandledMessage, this);
-    this._prevKernelName = session.kernel.name;
+
+    if (session) {
+      if (session.path !== this._path) {
+        this._path = session.path;
+        this._propertyChanged.emit('path');
+      }
+      if (session.name !== this._name) {
+        this._name = session.name;
+        this._propertyChanged.emit('name');
+      }
+      if (session.type !== this._type) {
+        this._type = session.type;
+        this._propertyChanged.emit('type');
+      }
+
+      session.terminated.connect(this._onTerminated, this);
+      session.propertyChanged.connect(this._onPropertyChanged, this);
+      session.kernelChanged.connect(this._onKernelChanged, this);
+      session.statusChanged.connect(this._onStatusChanged, this);
+      session.iopubMessage.connect(this._onIopubMessage, this);
+      session.unhandledMessage.connect(this._onUnhandledMessage, this);
+      this._prevKernelName = session.kernel.name;
+    }
     this._pendingKernelName = '';
 
     // The session kernel was disposed above when the session was disposed, so
     // the oldValue should be null.
-    this._kernelChanged.emit({ oldValue: null, newValue: session.kernel });
+    this._kernelChanged.emit({
+      oldValue: null,
+      newValue: session && session.kernel
+    });
     this._statusChanged.emit('unknown');
-    return session.kernel;
+
+    return session && session.kernel;
   }
 
   /**
@@ -768,6 +775,7 @@ export class ClientSession implements IClientSession {
   private async _handleSessionError(
     err: ServerConnection.ResponseError
   ): Promise<void> {
+    this._handleNewSession(null);
     let traceback = '';
     let message = '';
     try {
@@ -788,7 +796,6 @@ export class ClientSession implements IClientSession {
         )}
       </div>
     );
-
     const dialog = (this._dialog = new Dialog({
       title: 'Error Starting Kernel',
       body,

packages/services/src/kernel/default.ts

@@ -1245,7 +1245,10 @@ export class DefaultKernel implements Kernel.IKernel {
       .catch(error => {
         // Log any errors in handling the message, thus resetting the _msgChain
         // promise so we can process more messages.
-        console.error(error);
+        // Ignore the "Canceled" errors that are thrown during kernel dispose.
+        if (error.message.startsWith('Canceled future for ')) {
+          console.error(error);
+        }
       });
 
     // Emit the message receive signal

packages/services/src/kernel/future.ts

@@ -171,7 +171,11 @@ export abstract class KernelFutureHandler<
       // is waiting for the promise to resolve. This prevents the error from
       // being displayed in the console, but does not prevent it from being
       // caught by a client who is waiting for it.
-      this._done.reject(new Error('Canceled'));
+      this._done.reject(
+        new Error(
+          `Canceled future for ${this.msg.header.msg_type} message before replies were done`
+        )
+      );
       this._done.promise.catch(() => {
         /* no-op */
       });

tests/test-apputils/src/clientsession.spec.ts

@@ -311,6 +311,24 @@ describe('@jupyterlab/apputils', () => {
         // could be either, just make sure it isn't the original kernel.
         expect(lastKernel).to.be.oneOf([results[0], results[1]]);
       });
+
+      it('should handle an error during kernel change', async () => {
+        await session.initialize();
+        await session.kernel.ready;
+        let status = 'idle';
+        session.statusChanged.connect(() => {
+          status = session.status;
+        });
+        let caught = false;
+        const promise = session
+          .changeKernel({ name: 'does-not-exist' })
+          .catch(() => {
+            caught = true;
+          });
+        await Promise.all([promise, acceptDialog(document.body, 1000)]);
+        expect(caught).to.equal(true);
+        expect(status).to.equal('unknown');
+      });
     });
 
     describe('#selectKernel()', () => {