From cfc234dc89d7f3490bd33a3ccbc5b2546a0f40ed Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Fri, 26 Sep 2014 17:41:21 -0700 Subject: [PATCH 01/11] Handle NoSuchKernel errors more gracefully --- IPython/html/services/sessions/handlers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index 98def21a5..603dabcc5 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -10,6 +10,7 @@ from tornado import web from ...base.handlers import IPythonHandler, json_errors from IPython.utils.jsonutil import date_default from IPython.html.utils import url_path_join, url_escape +from IPython.kernel.kernelspec import NoSuchKernel class SessionRootHandler(IPythonHandler): @@ -52,7 +53,11 @@ class SessionRootHandler(IPythonHandler): if sm.session_exists(name=name, path=path): model = sm.get_session(name=name, path=path) else: - model = sm.create_session(name=name, path=path, kernel_name=kernel_name) + try: + model = sm.create_session(name=name, path=path, kernel_name=kernel_name) + except NoSuchKernel: + raise web.HTTPError(400, "No such kernel: %s" % kernel_name) + location = url_path_join(self.base_url, 'api', 'sessions', model['id']) self.set_header('Location', url_escape(location)) self.set_status(201) From 58fcb3abb9ec2b2287e078479ad8d1aa542a8600 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Fri, 26 Sep 2014 17:41:45 -0700 Subject: [PATCH 02/11] Show the user a different notification --- .../static/notebook/js/notificationarea.js | 19 ++++++++++++++++++- .../html/static/services/kernels/js/kernel.js | 1 + 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/IPython/html/static/notebook/js/notificationarea.js b/IPython/html/static/notebook/js/notificationarea.js index 81f650bfe..5c231c283 100644 --- a/IPython/html/static/notebook/js/notificationarea.js +++ b/IPython/html/static/notebook/js/notificationarea.js @@ -159,7 +159,7 @@ define([ }); }); - this.events.on('status_dead.Kernel',function () { + this.events.on('status_restart_failed.Kernel',function () { var msg = 'The kernel has died, and the automatic restart has failed.' + ' It is possible the kernel cannot be restarted.' + ' If you are not able to restart the kernel, you will still be able to save' + @@ -184,6 +184,23 @@ define([ }); }); + this.events.on('start_failed.Session',function () { + var msg = 'We were unable to start the kernel. This might ' + + 'happen if the notebook was previously run with a kernel ' + + 'that you do not have installed. Please choose a different kernel, ' + + 'or install the needed kernel and then refresh this page.'; + + dialog.modal({ + title: "Failed to start the kernel", + body : msg, + keyboard_manager: that.keyboard_manager, + notebook: that.notebook, + buttons : { + "Ok": { class: 'btn-primary' } + } + }); + }); + this.events.on('websocket_closed.Kernel', function (event, data) { var kernel = data.kernel; var ws_url = data.ws_url; diff --git a/IPython/html/static/services/kernels/js/kernel.js b/IPython/html/static/services/kernels/js/kernel.js index 9b4937a22..e1ec7e4e9 100644 --- a/IPython/html/static/services/kernels/js/kernel.js +++ b/IPython/html/static/services/kernels/js/kernel.js @@ -560,6 +560,7 @@ define([ } else if (execution_state === 'dead') { this.stop_channels(); this.events.trigger('status_dead.Kernel', {kernel: this}); + this.events.trigger('status_restart_failed.Kernel', {kernel: this}); } }; From 263181c4164bf9f5545d9f88bd82eae49aefa266 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Fri, 26 Sep 2014 18:40:14 -0700 Subject: [PATCH 03/11] Report the exact error that occurred --- .../static/notebook/js/notificationarea.js | 22 ++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/IPython/html/static/notebook/js/notificationarea.js b/IPython/html/static/notebook/js/notificationarea.js index 5c231c283..feef3da0b 100644 --- a/IPython/html/static/notebook/js/notificationarea.js +++ b/IPython/html/static/notebook/js/notificationarea.js @@ -184,11 +184,23 @@ define([ }); }); - this.events.on('start_failed.Session',function () { - var msg = 'We were unable to start the kernel. This might ' + - 'happen if the notebook was previously run with a kernel ' + - 'that you do not have installed. Please choose a different kernel, ' + - 'or install the needed kernel and then refresh this page.'; + this.events.on('start_failed.Session',function (session, xhr, status, error) { + var msg = $('
'); + msg.append($('
') + .text('We were unable to start the kernel. This might ' + + 'happen if the notebook was previously run with a kernel ' + + 'that you do not have installed. Please choose a different kernel, ' + + 'or install the needed kernel and then refresh this page.') + .css('margin-bottom', '1em')); + + msg.append($('
') + .text('The exact error was:') + .css('margin-bottom', '1em')); + + msg.append($('
') + .attr('class', 'alert alert-danger') + .attr('role', 'alert') + .text(JSON.parse(status.responseText).message)); dialog.modal({ title: "Failed to start the kernel", From 5ba858fc7caa1df5adeace77335ed4be62b55568 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Fri, 26 Sep 2014 18:58:09 -0700 Subject: [PATCH 04/11] Remove 'we' from message --- IPython/html/static/notebook/js/notificationarea.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/IPython/html/static/notebook/js/notificationarea.js b/IPython/html/static/notebook/js/notificationarea.js index feef3da0b..c2f9837ee 100644 --- a/IPython/html/static/notebook/js/notificationarea.js +++ b/IPython/html/static/notebook/js/notificationarea.js @@ -187,7 +187,7 @@ define([ this.events.on('start_failed.Session',function (session, xhr, status, error) { var msg = $('
'); msg.append($('
') - .text('We were unable to start the kernel. This might ' + + .text('The kernel could not be started. This might ' + 'happen if the notebook was previously run with a kernel ' + 'that you do not have installed. Please choose a different kernel, ' + 'or install the needed kernel and then refresh this page.') From c4a89cd54dd365e383a6a0ef568d3648797f92a5 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Fri, 26 Sep 2014 19:22:30 -0700 Subject: [PATCH 05/11] Better user experience when kernel isn't found --- IPython/html/services/sessions/handlers.py | 6 ++- .../static/notebook/js/notificationarea.js | 38 +++++++------------ .../static/services/sessions/js/session.js | 1 - 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index 603dabcc5..8b4a6d3e6 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -56,7 +56,11 @@ class SessionRootHandler(IPythonHandler): try: model = sm.create_session(name=name, path=path, kernel_name=kernel_name) except NoSuchKernel: - raise web.HTTPError(400, "No such kernel: %s" % kernel_name) + msg = ("The '%s' kernel is not available. Please pick another " + "suitable kernel instead, or install that kernel." % kernel_name) + status_msg = 'Kernel not found' + msg = dict(full=msg, short=status_msg) + raise web.HTTPError(400, json.dumps(msg)) location = url_path_join(self.base_url, 'api', 'sessions', model['id']) self.set_header('Location', url_escape(location)) diff --git a/IPython/html/static/notebook/js/notificationarea.js b/IPython/html/static/notebook/js/notificationarea.js index c2f9837ee..a87e3e8cc 100644 --- a/IPython/html/static/notebook/js/notificationarea.js +++ b/IPython/html/static/notebook/js/notificationarea.js @@ -185,31 +185,21 @@ define([ }); this.events.on('start_failed.Session',function (session, xhr, status, error) { - var msg = $('
'); - msg.append($('
') - .text('The kernel could not be started. This might ' + - 'happen if the notebook was previously run with a kernel ' + - 'that you do not have installed. Please choose a different kernel, ' + - 'or install the needed kernel and then refresh this page.') - .css('margin-bottom', '1em')); + var msg = JSON.parse(status.responseJSON.message); - msg.append($('
') - .text('The exact error was:') - .css('margin-bottom', '1em')); - - msg.append($('
') - .attr('class', 'alert alert-danger') - .attr('role', 'alert') - .text(JSON.parse(status.responseText).message)); - - dialog.modal({ - title: "Failed to start the kernel", - body : msg, - keyboard_manager: that.keyboard_manager, - notebook: that.notebook, - buttons : { - "Ok": { class: 'btn-primary' } - } + that.save_widget.update_document_title(); + $kernel_ind_icon.attr('class','kernel_dead_icon').attr('title','Kernel Dead'); + knw.danger(msg.short, undefined, function () { + dialog.modal({ + title: "Failed to start the kernel", + body : msg.full, + keyboard_manager: that.keyboard_manager, + notebook: that.notebook, + buttons : { + "Ok": { class: 'btn-primary' } + } + }); + return false; }); }); diff --git a/IPython/html/static/services/sessions/js/session.js b/IPython/html/static/services/sessions/js/session.js index 14a93af70..d8d5b5663 100644 --- a/IPython/html/static/services/sessions/js/session.js +++ b/IPython/html/static/services/sessions/js/session.js @@ -112,7 +112,6 @@ define([ Session.prototype._handle_start_failure = function (xhr, status, error) { this.events.trigger('start_failed.Session', [this, xhr, status, error]); - this.events.trigger('status_dead.Kernel'); }; /** From 5e1e8a116c9aa294b97307a83149d49b8893bd93 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Tue, 30 Sep 2014 10:21:25 -0700 Subject: [PATCH 06/11] Use 501 error code instead of 400 --- IPython/html/services/sessions/handlers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index 8b4a6d3e6..f38ad5859 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -60,7 +60,7 @@ class SessionRootHandler(IPythonHandler): "suitable kernel instead, or install that kernel." % kernel_name) status_msg = 'Kernel not found' msg = dict(full=msg, short=status_msg) - raise web.HTTPError(400, json.dumps(msg)) + raise web.HTTPError(501, json.dumps(msg)) location = url_path_join(self.base_url, 'api', 'sessions', model['id']) self.set_header('Location', url_escape(location)) From 46e40e5ea1490fa85b6c4b019485465b90e9e7ea Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Tue, 30 Sep 2014 10:48:43 -0700 Subject: [PATCH 07/11] Return a proper JSON object --- IPython/html/services/sessions/handlers.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/IPython/html/services/sessions/handlers.py b/IPython/html/services/sessions/handlers.py index f38ad5859..0573e6932 100644 --- a/IPython/html/services/sessions/handlers.py +++ b/IPython/html/services/sessions/handlers.py @@ -59,8 +59,10 @@ class SessionRootHandler(IPythonHandler): msg = ("The '%s' kernel is not available. Please pick another " "suitable kernel instead, or install that kernel." % kernel_name) status_msg = 'Kernel not found' - msg = dict(full=msg, short=status_msg) - raise web.HTTPError(501, json.dumps(msg)) + self.log.warn('Kernel not found: %s' % kernel_name) + self.set_status(501) + self.finish(json.dumps(dict(message=msg, short_message=status_msg))) + return location = url_path_join(self.base_url, 'api', 'sessions', model['id']) self.set_header('Location', url_escape(location)) From d0e942213d3f4a543e54ade31a5eaa8e67fb63f6 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Tue, 30 Sep 2014 10:48:54 -0700 Subject: [PATCH 08/11] Always show the modal dialog, and have a fallback generic message --- .../static/notebook/js/notificationarea.js | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/IPython/html/static/notebook/js/notificationarea.js b/IPython/html/static/notebook/js/notificationarea.js index a87e3e8cc..f2e43a53a 100644 --- a/IPython/html/static/notebook/js/notificationarea.js +++ b/IPython/html/static/notebook/js/notificationarea.js @@ -185,14 +185,25 @@ define([ }); this.events.on('start_failed.Session',function (session, xhr, status, error) { - var msg = JSON.parse(status.responseJSON.message); + var full = status.responseJSON.message; + var short = status.responseJSON.short_message || 'Kernel error'; + var traceback = status.responseJSON.traceback; + var msg = $('
'); - that.save_widget.update_document_title(); - $kernel_ind_icon.attr('class','kernel_dead_icon').attr('title','Kernel Dead'); - knw.danger(msg.short, undefined, function () { + msg.append($('

').text(full)); + if (traceback) { + msg.append($('