From 89df33012942cc1eddbca14f7cd70d4b36894930 Mon Sep 17 00:00:00 2001 From: Kester Tong Date: Tue, 28 Oct 2014 21:03:44 -0400 Subject: [PATCH] Modifies Contents API to return Error objects Modfies the Contents class to return JavaScript Error objects instead of passing on the return values from $.ajax(). This has two advantages. First, it allows the content manager to parse errors and give more informative messages than the ajax response. Second, it makes the Contents interface more general, since other kinds of backends might generate client-side errors. --- IPython/html/static/base/js/utils.js | 18 ++++++ IPython/html/static/notebook/js/menubar.js | 12 +--- IPython/html/static/notebook/js/notebook.js | 70 ++++++++------------- IPython/html/static/services/contents.js | 67 +++++++++++++++----- IPython/html/static/tree/js/main.js | 12 +--- IPython/html/static/tree/js/notebooklist.js | 5 +- 6 files changed, 106 insertions(+), 78 deletions(-) diff --git a/IPython/html/static/base/js/utils.js b/IPython/html/static/base/js/utils.js index 22261f3e4..16f9a7068 100644 --- a/IPython/html/static/base/js/utils.js +++ b/IPython/html/static/base/js/utils.js @@ -563,6 +563,22 @@ define([ ); }; + /** Error type for wrapped XHR errors. */ + var XHR_ERROR = 'XhrError'; + + /** + * Wraps an AJAX error as an Error object. + */ + var wrap_ajax_error = function (jqXHR, status, error) { + var wrapped_error = new Error(ajax_error_msg(jqXHR)); + wrapped_error.name = XHR_ERROR; + // provide xhr response + wrapped_error.xhr = jqXHR; + wrapped_error.xhr_status = status; + wrapped_error.xhr_error = error; + return wrapped_error; + } + var utils = { regex_split : regex_split, uuid : uuid, @@ -588,6 +604,8 @@ define([ ajax_error_msg : ajax_error_msg, log_ajax_error : log_ajax_error, requireCodeMirrorMode : requireCodeMirrorMode, + XHR_ERROR : XHR_ERROR, + wrap_ajax_error : wrap_ajax_error }; // Backwards compatability. diff --git a/IPython/html/static/notebook/js/menubar.js b/IPython/html/static/notebook/js/menubar.js index 8e14bc2dc..01b09ae0e 100644 --- a/IPython/html/static/notebook/js/menubar.js +++ b/IPython/html/static/notebook/js/menubar.js @@ -91,23 +91,17 @@ define([ // notebook's path. that.contents.new_notebook(that.notebook.notebook_path, { - success: function (data, status, xhr) { + success: function (data) { window.open( utils.url_join_encode( that.base_url, 'notebooks', data.path, data.name ), '_blank'); }, - error: function(xhr, status, error) { - var msg; - if (xhr.responseJSON && xhr.responseJSON.message) { - msg = xhr.responseJSON.message; - } else { - msg = xhr.statusText; - } + error: function(error) { dialog.modal({ title : 'Creating Notebook Failed', - body : "The error was: " + msg, + body : "The error was: " + error.message, buttons : {'OK' : {'class' : 'btn-primary'}} }); } diff --git a/IPython/html/static/notebook/js/notebook.js b/IPython/html/static/notebook/js/notebook.js index fdc1c615b..05d1c06aa 100644 --- a/IPython/html/static/notebook/js/notebook.js +++ b/IPython/html/static/notebook/js/notebook.js @@ -1923,7 +1923,7 @@ define([ this.contents.save_file(this.notebook_path, this.notebook_name, model, { extra_settings: extra_settings, success: $.proxy(this.save_notebook_success, this, start), - error: function (xhr, status, error) { + error: function (error) { that.events.trigger('notebook_save_failed.Notebook'); } }); @@ -1935,10 +1935,8 @@ define([ * @method save_notebook_success * @param {Integer} start Time when the save request start * @param {Object} data JSON representation of a notebook - * @param {String} status Description of response status - * @param {jqXHR} xhr jQuery Ajax object */ - Notebook.prototype.save_notebook_success = function (start, data, status, xhr) { + Notebook.prototype.save_notebook_success = function (start, data) { this.set_dirty(false); if (data.message) { // save succeeded, but validation failed. @@ -2078,7 +2076,7 @@ define([ var that = this; this.contents.rename_file(this.notebook_path, this.notebook_name, this.notebook_path, new_name, { - success: function (json, status, xhr) { + success: function (json) { var name = that.notebook_name = json.name; that.session.rename_notebook(name, json.path); that.events.trigger('notebook_renamed.Notebook', json); @@ -2091,12 +2089,12 @@ define([ this.contents.delete_file(this.notebook_name, this.notebook_path); }; - Notebook.prototype.rename_error = function (xhr, status, error) { + Notebook.prototype.rename_error = function (error) { var that = this; var dialog_body = $('
').append( $("

").text('This notebook name already exists.') ); - this.events.trigger('notebook_rename_failed.Notebook', [xhr, status, error]); + this.events.trigger('notebook_rename_failed.Notebook', error); dialog.modal({ notebook: this, keyboard_manager: this.keyboard_manager, @@ -2146,10 +2144,8 @@ define([ * * @method load_notebook_success * @param {Object} data JSON representation of a notebook - * @param {String} status Description of response status - * @param {jqXHR} xhr jQuery Ajax object */ - Notebook.prototype.load_notebook_success = function (data, status, xhr) { + Notebook.prototype.load_notebook_success = function (data) { var failed; try { this.fromJSON(data); @@ -2281,20 +2277,18 @@ define([ * Failure callback for loading a notebook from the server. * * @method load_notebook_error - * @param {jqXHR} xhr jQuery Ajax object - * @param {String} status Description of response status - * @param {String} error HTTP error message - */ - Notebook.prototype.load_notebook_error = function (xhr, status, error) { - this.events.trigger('notebook_load_failed.Notebook', [xhr, status, error]); - utils.log_ajax_error(xhr, status, error); - var msg = $("

"); - if (xhr.status === 400) { - msg.text(utils.ajax_error_msg(xhr)); - } else if (xhr.status === 500) { - msg.text("An unknown error occurred while loading this notebook. " + + * @param {Error} error + */ + Notebook.prototype.load_notebook_error = function (error) { + this.events.trigger('notebook_load_failed.Notebook', error); + var msg; + if (error.name = utils.XHR_ERROR && error.xhr.status === 500) { + utils.log_ajax_error(error.xhr, error.xhr_status, error.xhr_error); + msg = "An unknown error occurred while loading this notebook. " + "This version can load notebook formats " + - "v" + this.nbformat + " or earlier. See the server log for details."); + "v" + this.nbformat + " or earlier. See the server log for details."; + } else { + msg = error.message; } dialog.modal({ notebook: this, @@ -2350,7 +2344,7 @@ define([ var that = this; this.contents.list_checkpoints(this.notebook_path, this.notebook_name, { success: $.proxy(this.list_checkpoints_success, this), - error: function(xhr, status, error_msg) { + error: function(error) { that.events.trigger('list_checkpoints_failed.Notebook'); } }); @@ -2361,10 +2355,8 @@ define([ * * @method list_checkpoint_success * @param {Object} data JSON representation of a checkpoint - * @param {String} status Description of response status - * @param {jqXHR} xhr jQuery Ajax object */ - Notebook.prototype.list_checkpoints_success = function (data, status, xhr) { + Notebook.prototype.list_checkpoints_success = function (data) { data = $.parseJSON(data); this.checkpoints = data; if (data.length) { @@ -2384,7 +2376,7 @@ define([ var that = this; this.contents.create_checkpoint(this.notebook_path, this.notebook_name, { success: $.proxy(this.create_checkpoint_success, this), - error: function (xhr, status, error_msg) { + error: function (error) { that.events.trigger('checkpoint_failed.Notebook'); } }); @@ -2395,10 +2387,8 @@ define([ * * @method create_checkpoint_success * @param {Object} data JSON representation of a checkpoint - * @param {String} status Description of response status - * @param {jqXHR} xhr jQuery Ajax object */ - Notebook.prototype.create_checkpoint_success = function (data, status, xhr) { + Notebook.prototype.create_checkpoint_success = function (data) { data = $.parseJSON(data); this.add_checkpoint(data); this.events.trigger('checkpoint_created.Notebook', data); @@ -2457,7 +2447,7 @@ define([ this.contents.restore_checkpoint(this.notebook_path, this.notebook_name, checkpoint, { success: $.proxy(this.create_checkpoint_success, this), - error: function (xhr, status, error_msg) { + error: function (error) { that.events.trigger('checkpoint_restore_failed.Notebook'); } }); @@ -2467,11 +2457,8 @@ define([ * Success callback for restoring a notebook to a checkpoint. * * @method restore_checkpoint_success - * @param {Object} data (ignored, should be empty) - * @param {String} status Description of response status - * @param {jqXHR} xhr jQuery Ajax object */ - Notebook.prototype.restore_checkpoint_success = function (data, status, xhr) { + Notebook.prototype.restore_checkpoint_success = function () { this.events.trigger('checkpoint_restored.Notebook'); this.load_notebook(this.notebook_name, this.notebook_path); }; @@ -2488,8 +2475,8 @@ define([ this.contents.delete_checkpoint(this.notebook_path, this.notebook_name, checkpoint, { success: $.proxy(this.create_checkpoint_success, this), - error: function (xhr, status, error_msg) { - that.events.trigger('checkpoint_delete_failed.Notebook', [xhr, status, error]); + error: function (error) { + that.events.trigger('checkpoint_delete_failed.Notebook', error); } }); }; @@ -2498,12 +2485,9 @@ define([ * Success callback for deleting a notebook checkpoint * * @method delete_checkpoint_success - * @param {Object} data (ignored, should be empty) - * @param {String} status Description of response status - * @param {jqXHR} xhr jQuery Ajax object */ - Notebook.prototype.delete_checkpoint_success = function (data, status, xhr) { - this.events.trigger('checkpoint_deleted.Notebook', data); + Notebook.prototype.delete_checkpoint_success = function () { + this.events.trigger('checkpoint_deleted.Notebook'); this.load_notebook(this.notebook_name, this.notebook_path); }; diff --git a/IPython/html/static/services/contents.js b/IPython/html/static/services/contents.js index 7dda2a3f0..69190316a 100644 --- a/IPython/html/static/services/contents.js +++ b/IPython/html/static/services/contents.js @@ -21,12 +21,47 @@ define([ this.base_url = options.base_url; }; + /** Error type */ + Contents.DIRECTORY_NOT_EMPTY_ERROR = 'DirectoryNotEmptyError'; + + Contents.DirectoryNotEmptyError = function() { + // Constructor + // + // An error representing the result of attempting to delete a non-empty + // directory. + this.message = 'A directory must be empty before being deleted.'; + } + Contents.DirectoryNotEmptyError.prototype = new Error; + Contents.DirectoryNotEmptyError.prototype.name = + Contents.DIRECTORY_NOTE_EMPTY_ERROR; + + Contents.prototype.api_url = function() { var url_parts = [this.base_url, 'api/contents'].concat( Array.prototype.slice.apply(arguments)); return utils.url_join_encode.apply(null, url_parts); }; + /** + * Creates a basic error handler that wraps a jqXHR error as an Error. + * + * Takes a callback that accepts an Error, and returns a callback that can + * be passed directly to $.ajax, which will wrap the error from jQuery + * as an Error, and pass that to the original callback. + * + * @method create_basic_error_handler + * @param{Function} callback + * @return{Function} + */ + Contents.prototype.create_basic_error_handler = function(callback) { + if (!callback) { + return function(xhr, status, error) { }; + } + return function(xhr, status, error) { + callback(utils.wrap_ajax_error(xhr, status, error)); + }; + } + /** * File Functions (including notebook operations) */ @@ -50,7 +85,7 @@ define([ type : "GET", dataType : "json", success : options.success, - error : options.error || function() {} + error : this.create_basic_error_handler(options.error) }; var url = this.api_url(path, name); $.ajax(url, settings); @@ -71,7 +106,7 @@ define([ type : "POST", dataType : "json", success : options.success || function() {}, - error : options.error || function() {} + error : this.create_basic_error_handler(options.error) }; $.ajax(this.api_url(path), settings); }; @@ -86,8 +121,12 @@ define([ dataType : "json", success : options.success || function() {}, error : function(xhr, status, error) { - utils.log_ajax_error(xhr, status, error); - error(xhr, status, error); + // TODO: update IPEP27 to specify errors more precisely, so + // that error types can be detected here with certainty. + if (xhr.status === 400) { + error(new Contents.DirectoryNotEmptyError()); + } + error(utils.wrap_ajax_error(xhr, status, error)); } }; var url = this.api_url(path, name); @@ -103,8 +142,8 @@ define([ data : JSON.stringify(data), dataType: "json", contentType: 'application/json', - success : options.success || function() {}, - error : options.error || function() {} + success : options.success || function() {}, + error : this.create_basic_error_handler(options.error) }; var url = this.api_url(path, name); $.ajax(url, settings); @@ -119,7 +158,7 @@ define([ data : JSON.stringify(model), contentType: 'application/json', success : options.success || function() {}, - error : options.error || function() {} + error : this.create_basic_error_handler(options.error) }; if (options.extra_settings) { $.extend(settings, options.extra_settings); @@ -137,7 +176,7 @@ define([ var settings = { type : "POST", success: options.success || function() {}, - error: options.error || function() {} + error : this.create_basic_error_handler(options.error) }; $.ajax(url, settings); }; @@ -147,7 +186,7 @@ define([ var settings = { type : "GET", success: options.success, - error: options.error || function() {} + error : this.create_basic_error_handler(options.error) }; $.ajax(url, settings); }; @@ -157,17 +196,17 @@ define([ var settings = { type : "POST", success: options.success || function() {}, - error: options.error || function() {} + error : this.create_basic_error_handler(options.error) }; $.ajax(url, settings); }; - Contents.prototype.delete_checkpoint = function(path, name, checkpoint_id, options) { + Contents.prototype.delete_checkpoint = function(path, name, checkpoint_id, options) { var url = this.api_url(path, name, 'checkpoints', checkpoint_id); var settings = { type : "DELETE", success: options.success || function() {}, - error: options.error || function() {} + error : this.create_basic_error_handler(options.error) }; $.ajax(url, settings); }; @@ -199,7 +238,7 @@ define([ type : "GET", dataType : "json", success : options.success, - error : options.error || function() {} + error : this.create_basic_error_handler(options.error) }; $.ajax(this.api_url(path), settings); @@ -209,4 +248,4 @@ define([ IPython.Contents = Contents; return {'Contents': Contents}; -}); +}); diff --git a/IPython/html/static/tree/js/main.js b/IPython/html/static/tree/js/main.js index 1f53a7f77..73ed8b18e 100644 --- a/IPython/html/static/tree/js/main.js +++ b/IPython/html/static/tree/js/main.js @@ -62,23 +62,17 @@ require([ $('#new_notebook').click(function (e) { contents.new_notebook(common_options.notebook_path, { - success: function (data, status, xhr) { + success: function (data) { window.open( utils.url_join_encode( common_options.base_url, 'notebooks', data.path, data.name ), '_blank'); }, - error: function(xhr, status, error) { - var msg; - if (xhr.responseJSON && xhr.responseJSON.message) { - msg = xhr.responseJSON.message; - } else { - msg = xhr.statusText; - } + error: function(error) { dialog.modal({ title : 'Creating Notebook Failed', - body : "The error was: " + msg, + body : "The error was: " + error.message, buttons : {'OK' : {'class' : 'btn-primary'}} }); } diff --git a/IPython/html/static/tree/js/notebooklist.js b/IPython/html/static/tree/js/notebooklist.js index 2e62f40b5..a044025e2 100644 --- a/IPython/html/static/tree/js/notebooklist.js +++ b/IPython/html/static/tree/js/notebooklist.js @@ -144,9 +144,8 @@ define([ var that = this this.contents.list_contents(that.notebook_path, { success: $.proxy(this.draw_notebook_list, this), - error: function(xhr, status, error) { - utils.log_ajax_error(xhr, status, error); - that.draw_notebook_list([], "Error connecting to server."); + error: function(error) { + that.draw_notebook_list([], "Server error: " + error.message); } }); };