From ef406a25a1c9ee18bbaa8e9ca5e7f0886c62c5bb Mon Sep 17 00:00:00 2001 From: Nik Wakelin Date: Fri, 4 Dec 2015 18:07:37 +0000 Subject: [PATCH] Adds more detailed ("friendly") RESTAdapter Error Messages --- addon/adapters/rest-adapter.js | 48 ++++++++++++-- .../integration/adapter/rest-adapter-test.js | 66 ++++++++++++++++++- .../rest-adapter/detailed-message-test.js | 51 ++++++++++++++ 3 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 tests/unit/adapters/rest-adapter/detailed-message-test.js diff --git a/addon/adapters/rest-adapter.js b/addon/adapters/rest-adapter.js index 26098e3eb58..f2e1906354a 100644 --- a/addon/adapters/rest-adapter.js +++ b/addon/adapters/rest-adapter.js @@ -743,18 +743,20 @@ export default Adapter.extend(BuildURLMixin, { @param {Number} status @param {Object} headers @param {Object} payload + @param {Object} requestData - the original request information @return {Object | DS.AdapterError} response */ - handleResponse(status, headers, payload) { + handleResponse(status, headers, payload, requestData) { if (this.isSuccess(status, headers, payload)) { return payload; } else if (this.isInvalid(status, headers, payload)) { return new InvalidError(payload.errors); } - let errors = this.normalizeErrorResponse(status, headers, payload); + let errors = this.normalizeErrorResponse(status, headers, payload); + let detailedMessage = this.generatedDetailedMessage(status, headers, payload, requestData); - return new AdapterError(errors); + return new AdapterError(errors, detailedMessage); }, /** @@ -812,6 +814,11 @@ export default Adapter.extend(BuildURLMixin, { ajax(url, type, options) { var adapter = this; + var requestData = { + url: url, + method: type + }; + return new Ember.RSVP.Promise(function(resolve, reject) { var hash = adapter.ajaxOptions(url, type, options); @@ -820,7 +827,8 @@ export default Adapter.extend(BuildURLMixin, { let response = adapter.handleResponse( jqXHR.status, parseResponseHeaders(jqXHR.getAllResponseHeaders()), - payload + payload, + requestData ); if (response instanceof AdapterError) { @@ -844,7 +852,8 @@ export default Adapter.extend(BuildURLMixin, { error = adapter.handleResponse( jqXHR.status, parseResponseHeaders(jqXHR.getAllResponseHeaders()), - adapter.parseErrorResponse(jqXHR.responseText) || errorThrown + adapter.parseErrorResponse(jqXHR.responseText) || errorThrown, + requestData ); } } @@ -922,6 +931,35 @@ export default Adapter.extend(BuildURLMixin, { } ]; } + }, + + /** + Generates a detailed ("friendly") error message, with plenty + of information for debugging (good luck!) + + @method generatedDetailedMessage + @private + @param {Number} status + @param {Object} headers + @param {Object} payload + @return {Object} request information + */ + generatedDetailedMessage: function(status, headers, payload, requestData) { + var shortenedPayload; + var payloadContentType = headers["Content-Type"] || "Empty Content-Type"; + + if (payloadContentType === "text/html" && payload.length > 250) { + shortenedPayload = "[Omitted Lengthy HTML]"; + } else { + shortenedPayload = payload; + } + + var requestDescription = requestData.method + ' ' + requestData.url; + var payloadDescription = 'Payload (' + payloadContentType + ')'; + + return ['Ember Data Request ' + requestDescription + ' returned a ' + status, + payloadDescription, + shortenedPayload].join('\n'); } }); diff --git a/tests/integration/adapter/rest-adapter-test.js b/tests/integration/adapter/rest-adapter-test.js index 4d99db2543c..de6fefa4afc 100644 --- a/tests/integration/adapter/rest-adapter-test.js +++ b/tests/integration/adapter/rest-adapter-test.js @@ -2194,8 +2194,8 @@ test("calls adapter.handleResponse with the jqXHR and json", function(assert) { } }); -test('calls handleResponse with jqXHR, jqXHR.responseText', function(assert) { - assert.expect(3); +test('calls handleResponse with jqXHR, jqXHR.responseText, and requestData', function(assert) { + assert.expect(4); var originalAjax = Ember.$.ajax; var jqXHR = { status: 400, @@ -2203,13 +2203,19 @@ test('calls handleResponse with jqXHR, jqXHR.responseText', function(assert) { getAllResponseHeaders() { return ''; } }; + var expectedRequestData = { + method: "GET", + url: "/posts/1" + }; + Ember.$.ajax = function(hash) { hash.error(jqXHR, jqXHR.responseText, 'Bad Request'); }; - adapter.handleResponse = function(status, headers, json) { + adapter.handleResponse = function(status, headers, json, requestData) { assert.deepEqual(status, 400); assert.deepEqual(json, jqXHR.responseText); + assert.deepEqual(requestData, expectedRequestData); return new DS.AdapterError('nope!'); }; @@ -2312,6 +2318,60 @@ test('on error wraps the error string in an DS.AdapterError object', function(as } }); +test('error handling includes a detailed message from the server', (assert) => { + assert.expect(2); + + let originalAjax = Ember.$.ajax; + let jqXHR = { + status: 500, + responseText: 'An error message, perhaps generated from a backend server!', + getAllResponseHeaders: function() { return 'Content-Type: text/plain'; } + }; + + Ember.$.ajax = function(hash) { + hash.error(jqXHR, 'error'); + }; + + try { + run(function() { + store.find('post', '1').catch(function(err) { + assert.equal(err.message, "Ember Data Request GET /posts/1 returned a 500\nPayload (text/plain)\nAn error message, perhaps generated from a backend server!"); + assert.ok(err, 'promise rejected'); + }); + }); + } finally { + Ember.$.ajax = originalAjax; + } + +}); + +test('error handling with a very long HTML-formatted payload truncates the friendly message', (assert) => { + assert.expect(2); + + let originalAjax = Ember.$.ajax; + let jqXHR = { + status: 500, + responseText: new Array(100).join(""), + getAllResponseHeaders: function() { return 'Content-Type: text/html'; } + }; + + Ember.$.ajax = function(hash) { + hash.error(jqXHR, 'error'); + }; + + try { + run(function() { + store.find('post', '1').catch(function(err) { + assert.equal(err.message, "Ember Data Request GET /posts/1 returned a 500\nPayload (text/html)\n[Omitted Lengthy HTML]"); + assert.ok(err, 'promise rejected'); + }); + }); + } finally { + Ember.$.ajax = originalAjax; + } + +}); + test('findAll resolves with a collection of DS.Models, not DS.InternalModels', (assert) => { assert.expect(4); diff --git a/tests/unit/adapters/rest-adapter/detailed-message-test.js b/tests/unit/adapters/rest-adapter/detailed-message-test.js new file mode 100644 index 00000000000..0078b54f1a0 --- /dev/null +++ b/tests/unit/adapters/rest-adapter/detailed-message-test.js @@ -0,0 +1,51 @@ +import setupStore from 'dummy/tests/helpers/store'; + +import {module, test} from 'qunit'; + +import DS from 'ember-data'; + +let adapter, env; + +module("unit/adapters/rest_adapter/detailed_message_test - DS.RESTAdapter#generatedDetailedMessage", { + + beforeEach() { + env = setupStore({ adapter: DS.RESTAdapter }); + adapter = env.adapter; + } + +}); + +test("generating a wonderfully friendly error message should work", (assert) => { + assert.expect(1); + + let friendlyMessage = adapter.generatedDetailedMessage( + 418, + { "Content-Type": "text/plain" }, + "I'm a little teapot, short and stout", + { + url: "/teapots/testing", + method: "GET" + } + ); + + assert.equal(friendlyMessage, ["Ember Data Request GET /teapots/testing returned a 418", + "Payload (text/plain)", + "I'm a little teapot, short and stout"].join("\n")); +}); + +test("generating a friendly error message with a missing content-type header should work", (assert) => { + + let friendlyMessage = adapter.generatedDetailedMessage( + 418, + {}, + "I'm a little teapot, short and stout", + { + url: "/teapots/testing", + method: "GET" + } + ); + + assert.equal(friendlyMessage, ["Ember Data Request GET /teapots/testing returned a 418", + "Payload (Empty Content-Type)", + "I'm a little teapot, short and stout"].join("\n")); +});