Browse Source

Fix session context error handling

Steven Silvester 5 years ago
parent
commit
37748629e0

+ 28 - 23
packages/apputils/src/sessioncontext.tsx

@@ -506,7 +506,7 @@ export class SessionContext implements ISessionContext {
     return (
       (kernel?.connectionStatus === 'connected'
         ? kernel?.status
-        : kernel?.connectionStatus) ?? ''
+        : kernel?.connectionStatus) ?? 'unknown'
     );
   }
 
@@ -782,7 +782,7 @@ export class SessionContext implements ISessionContext {
    * Handle a new session object.
    */
   private _handleNewSession(
-    session: Session.ISessionConnection
+    session: Session.ISessionConnection | null
   ): Kernel.IKernelConnection | null {
     if (this.isDisposed) {
       throw Error('Disposed');
@@ -795,28 +795,31 @@ export class SessionContext implements ISessionContext {
       this._session.dispose();
     }
     this._session = session;
-    this._prevKernelName = session.kernel?.name ?? '';
     this._pendingKernelName = '';
 
-    session.disposed.connect(this._onSessionDisposed, this);
-    session.propertyChanged.connect(this._onPropertyChanged, this);
-    session.kernelChanged.connect(this._onKernelChanged, this);
-    session.statusChanged.connect(this._onStatusChanged, this);
-    session.connectionStatusChanged.connect(
-      this._onConnectionStatusChanged,
-      this
-    );
-    session.iopubMessage.connect(this._onIopubMessage, this);
-    session.unhandledMessage.connect(this._onUnhandledMessage, this);
+    if (session) {
+      this._prevKernelName = session.kernel?.name ?? '';
 
-    if (session.path !== this._path) {
-      this._onPropertyChanged(session, 'path');
-    }
-    if (session.name !== this._name) {
-      this._onPropertyChanged(session, 'name');
-    }
-    if (session.type !== this._type) {
-      this._onPropertyChanged(session, 'type');
+      session.disposed.connect(this._onSessionDisposed, this);
+      session.propertyChanged.connect(this._onPropertyChanged, this);
+      session.kernelChanged.connect(this._onKernelChanged, this);
+      session.statusChanged.connect(this._onStatusChanged, this);
+      session.connectionStatusChanged.connect(
+        this._onConnectionStatusChanged,
+        this
+      );
+      session.iopubMessage.connect(this._onIopubMessage, this);
+      session.unhandledMessage.connect(this._onUnhandledMessage, this);
+
+      if (session.path !== this._path) {
+        this._onPropertyChanged(session, 'path');
+      }
+      if (session.name !== this._name) {
+        this._onPropertyChanged(session, 'name');
+      }
+      if (session.type !== this._type) {
+        this._onPropertyChanged(session, 'type');
+      }
     }
 
     // Any existing session/kernel connection was disposed above when the session was
@@ -828,11 +831,12 @@ export class SessionContext implements ISessionContext {
     });
     this._kernelChanged.emit({
       oldValue: null,
-      newValue: session.kernel,
+      newValue: session?.kernel || null,
       name: 'kernel'
     });
     this._statusChanged.emit(session?.kernel?.status || 'unknown');
-    return session.kernel;
+
+    return session?.kernel || null;
   }
 
   /**
@@ -841,6 +845,7 @@ export class SessionContext implements ISessionContext {
   private async _handleSessionError(
     err: ServerConnection.ResponseError
   ): Promise<void> {
+    this._handleNewSession(null);
     let traceback = '';
     let message = '';
     try {

+ 4 - 1
packages/services/src/kernel/default.ts

@@ -1420,7 +1420,10 @@ export class KernelConnection implements Kernel.IKernelConnection {
       .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

+ 18 - 0
tests/test-apputils/src/sessioncontext.spec.ts

@@ -421,6 +421,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 sessionContext.initialize();
+        await sessionContext.session?.kernel?.info;
+        let status = 'idle';
+        sessionContext.statusChanged.connect(() => {
+          status = sessionContext.kernelDisplayStatus;
+        });
+        let caught = false;
+        const promise = sessionContext
+          .changeKernel({ name: 'does-not-exist' })
+          .catch(() => {
+            caught = true;
+          });
+        await Promise.all([promise, acceptDialog()]);
+        expect(caught).to.equal(true);
+        expect(status).to.equal('unknown');
+      });
     });
 
     describe('#shutdown', () => {