From a715e4f994f91f41b33348c4a84d0ec2ff326120 Mon Sep 17 00:00:00 2001 From: Thomas Delnoij Date: Mon, 2 Nov 2015 14:38:54 +0100 Subject: [PATCH 1/6] Add error handler for vnd+error, support registering custom error handlers. --- src/context.js | 25 +++++++++++++++++- src/context.spec.js | 60 ++++++++++++++++++++++++++++++++++++++++++-- src/halerror.js | 36 ++++++++++++++++++++++++++ src/halerror.spec.js | 27 ++++++++++++++++++++ 4 files changed, 145 insertions(+), 3 deletions(-) create mode 100644 src/halerror.js create mode 100644 src/halerror.spec.js diff --git a/src/context.js b/src/context.js index 711355a..cef54a3 100644 --- a/src/context.js +++ b/src/context.js @@ -2,6 +2,10 @@ angular.module('hypermedia') + .config(function ($httpProvider) { + $httpProvider.interceptors.push('errorInterceptor'); + }) + /** * @ngdoc type * @name ResourceContext @@ -10,7 +14,7 @@ angular.module('hypermedia') * Context for working with hypermedia resources. The context has methods * for making HTTP requests and acts as an identity map. */ - .factory('ResourceContext', ['$http', '$log', '$q', 'Resource', function ($http, $log, $q, Resource) { + .factory('ResourceContext', ['$http', '$log', '$q', 'Resource', 'errorInterceptor', function ($http, $log, $q, Resource, errorInterceptor) { var busyRequests = 0; @@ -206,6 +210,10 @@ angular.module('hypermedia') */ busyRequests: {get: function () { return busyRequests; + }}, + + registerErrorHandler: {value: function (contentType, handler) { + errorInterceptor.registerErrorHandler(contentType, handler); }} }); @@ -229,6 +237,21 @@ angular.module('hypermedia') } }]) + .factory('errorInterceptor', function ($q) { + var handlers; + return { + 'responseError': function (response) { + var contentType = response.headers('Content-Type'); + var handler = handlers[contentType]; + return handler ? handler(response) : $q.reject(response); + }, + 'registerErrorHandler': function (contentType, handler) { + if (!handlers) handlers = {}; + handlers[contentType] = handler; + } + } + }) + ; /** diff --git a/src/context.spec.js b/src/context.spec.js index abce928..9a7fedc 100644 --- a/src/context.spec.js +++ b/src/context.spec.js @@ -6,11 +6,14 @@ describe('ResourceContext', function () { // Setup - var $httpBackend, ResourceContext, context, resource; + var $httpBackend, $q, ResourceContext, context, errorInterceptor, resource; + var problemJson = 'application/problem+json'; - beforeEach(inject(function (_$httpBackend_, _ResourceContext_) { + beforeEach(inject(function (_$httpBackend_, _$q_, _ResourceContext_, _errorInterceptor_) { $httpBackend = _$httpBackend_; + $q = _$q_; ResourceContext = _ResourceContext_; + errorInterceptor = _errorInterceptor_; context = new ResourceContext(); resource = context.get('http://example.com'); })); @@ -23,6 +26,59 @@ describe('ResourceContext', function () { // Tests + it('registers error handler', function () { + var func = function () {}; + spyOn(errorInterceptor, 'registerErrorHandler'); + + ResourceContext.registerErrorHandler(problemJson, func); + + expect(errorInterceptor.registerErrorHandler).toHaveBeenCalledWith(problemJson, func); + }); + + it('invokes error handler for content type', function () { + var spy = jasmine.createSpy('spy').and.callFake(function (response) { + return $q.reject(response); + }); + ResourceContext.registerErrorHandler(problemJson, spy); + + context.httpGet(resource); + $httpBackend.expectGET(resource.$uri, {'Accept': 'application/json'}) + .respond(500, null, {'Content-Type': problemJson}); + $httpBackend.flush(); + + expect(spy).toHaveBeenCalled(); + }); + + it('rejects response if no matching error handler', function () { + var promiseResult = null; + var spy = jasmine.createSpy('spy'); + ResourceContext.registerErrorHandler('application/json', spy); + + context.httpGet(resource).catch(function(result){ + promiseResult = result; + }); + $httpBackend.expectGET(resource.$uri, {'Accept': 'application/json'}) + .respond(500, {}, {'Content-Type': problemJson}); + $httpBackend.flush(); + + expect(spy).not.toHaveBeenCalled(); + expect(promiseResult.status).toBe(500); + }); + + it('invokes default error handler for content type \'application/vnd+error\'', function () { + var promiseResult; + var msg = 'Validatie fout'; + context.httpGet(resource).catch(function(result){ + promiseResult = result; + }); + $httpBackend.expectGET(resource.$uri, {'Accept': 'application/json'}) + .respond(500, {message: msg}, {'Content-Type': 'application/vnd+error'}); + $httpBackend.flush(); + + expect(promiseResult.error).toBeDefined(); + expect(promiseResult.error.message).toBe(msg); + }); + it('creates unique resources', function () { expect(context.get('http://example.com')).toBe(resource); expect(context.get('http://example.com/other')).not.toBe(resource); diff --git a/src/halerror.js b/src/halerror.js new file mode 100644 index 0000000..48aa9e3 --- /dev/null +++ b/src/halerror.js @@ -0,0 +1,36 @@ +'use strict'; + +angular.module('hypermedia') + + .run(function($q, ResourceContext, HalError){ + var vndErrorHandler = function (response) { + response.error = new HalError(response.data); + + return $q.reject(response); + }; + + ResourceContext.registerErrorHandler('application/vnd+error', vndErrorHandler); + }) + + .factory('HalError', function () { + var HalError = function (data) { + var self = this; + this.message = data.message; + this.errors = []; + + var embeds = (data._embedded ? data._embedded['ilent:error'] : undefined); + if (embeds) { + if (!Array.isArray(embeds)) { + embeds = [embeds]; + } + embeds.forEach(function (embed) { + self.errors.push(new HalError(embed)); + }); + } + }; + + return HalError; + }) + + +; \ No newline at end of file diff --git a/src/halerror.spec.js b/src/halerror.spec.js new file mode 100644 index 0000000..d212b12 --- /dev/null +++ b/src/halerror.spec.js @@ -0,0 +1,27 @@ +describe('HalError', function () { + beforeEach(module('hypermedia')); + + var HalError; + + beforeEach(inject(function (_HalError_) { + HalError = _HalError_; + })); + + it('can be constructed with embedded errors', function () { + var data = { + 'message': 'Validatie fout', + '_links': {'profile': {'href': 'http://nocarrier.co.uk/profiles/vnd.error/'}}, + '_embedded': { + 'ilent:error': { + 'message': 'Invalide nummer', + '_links': {'profile': {'href': 'http://nocarrier.co.uk/profiles/vnd.error/'}} + } + } + }; + + var error = new HalError(data); + + expect(error.message).toBe('Validatie fout'); + expect(error.errors.length).toBe(1); + }); +}); From a11e3e06209a279f454d6093fe716adf1481add3 Mon Sep 17 00:00:00 2001 From: Thomas Delnoij Date: Mon, 2 Nov 2015 14:49:22 +0100 Subject: [PATCH 2/6] Fix jshint --- src/context.js | 2 +- src/halerror.spec.js | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/context.js b/src/context.js index cef54a3..97d2671 100644 --- a/src/context.js +++ b/src/context.js @@ -249,7 +249,7 @@ angular.module('hypermedia') if (!handlers) handlers = {}; handlers[contentType] = handler; } - } + }; }) ; diff --git a/src/halerror.spec.js b/src/halerror.spec.js index d212b12..04db57b 100644 --- a/src/halerror.spec.js +++ b/src/halerror.spec.js @@ -1,3 +1,5 @@ +'use strict'; + describe('HalError', function () { beforeEach(module('hypermedia')); From 6347fa945851ec0cd11f62e921cef04a190c243b Mon Sep 17 00:00:00 2001 From: Thomas Delnoij Date: Mon, 2 Nov 2015 14:54:37 +0100 Subject: [PATCH 3/6] jscs --- src/context.spec.js | 4 ++-- src/halerror.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/context.spec.js b/src/context.spec.js index 9a7fedc..00d02a3 100644 --- a/src/context.spec.js +++ b/src/context.spec.js @@ -54,7 +54,7 @@ describe('ResourceContext', function () { var spy = jasmine.createSpy('spy'); ResourceContext.registerErrorHandler('application/json', spy); - context.httpGet(resource).catch(function(result){ + context.httpGet(resource).catch(function (result) { promiseResult = result; }); $httpBackend.expectGET(resource.$uri, {'Accept': 'application/json'}) @@ -68,7 +68,7 @@ describe('ResourceContext', function () { it('invokes default error handler for content type \'application/vnd+error\'', function () { var promiseResult; var msg = 'Validatie fout'; - context.httpGet(resource).catch(function(result){ + context.httpGet(resource).catch(function (result) { promiseResult = result; }); $httpBackend.expectGET(resource.$uri, {'Accept': 'application/json'}) diff --git a/src/halerror.js b/src/halerror.js index 48aa9e3..d972799 100644 --- a/src/halerror.js +++ b/src/halerror.js @@ -2,7 +2,7 @@ angular.module('hypermedia') - .run(function($q, ResourceContext, HalError){ + .run(function ($q, ResourceContext, HalError) { var vndErrorHandler = function (response) { response.error = new HalError(response.data); From 4e62f60805b83720d28b84d8f884aed312765855 Mon Sep 17 00:00:00 2001 From: Thomas Delnoij Date: Mon, 2 Nov 2015 14:56:49 +0100 Subject: [PATCH 4/6] jscs --- dist/hypermedia.js | 62 +++++++++++++++++++++++++++++++++++++++++++++- src/halerror.js | 2 +- 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/dist/hypermedia.js b/dist/hypermedia.js index 66d73d5..2023bd9 100644 --- a/dist/hypermedia.js +++ b/dist/hypermedia.js @@ -96,6 +96,10 @@ angular.module('hypermedia') angular.module('hypermedia') + .config(function ($httpProvider) { + $httpProvider.interceptors.push('errorInterceptor'); + }) + /** * @ngdoc type * @name ResourceContext @@ -104,7 +108,7 @@ angular.module('hypermedia') * Context for working with hypermedia resources. The context has methods * for making HTTP requests and acts as an identity map. */ - .factory('ResourceContext', ['$http', '$log', '$q', 'Resource', function ($http, $log, $q, Resource) { + .factory('ResourceContext', ['$http', '$log', '$q', 'Resource', 'errorInterceptor', function ($http, $log, $q, Resource, errorInterceptor) { var busyRequests = 0; @@ -300,6 +304,10 @@ angular.module('hypermedia') */ busyRequests: {get: function () { return busyRequests; + }}, + + registerErrorHandler: {value: function (contentType, handler) { + errorInterceptor.registerErrorHandler(contentType, handler); }} }); @@ -323,6 +331,21 @@ angular.module('hypermedia') } }]) + .factory('errorInterceptor', function ($q) { + var handlers; + return { + 'responseError': function (response) { + var contentType = response.headers('Content-Type'); + var handler = handlers[contentType]; + return handler ? handler(response) : $q.reject(response); + }, + 'registerErrorHandler': function (contentType, handler) { + if (!handlers) handlers = {}; + handlers[contentType] = handler; + } + }; + }) + ; /** @@ -336,6 +359,43 @@ angular.module('hypermedia') 'use strict'; +angular.module('hypermedia') + + .run(function ($q, ResourceContext, HalError) { + var vndErrorHandler = function (response) { + response.error = new HalError(response.data); + + return $q.reject(response); + }; + + ResourceContext.registerErrorHandler('application/vnd+error', vndErrorHandler); + }) + + .factory('HalError', function () { + var HalError = function (data) { + var self = this; + this.message = data.message; + this.errors = []; + + var embeds = (data._embedded ? data._embedded['ilent:error'] : undefined); + if (embeds) { + if (!Array.isArray(embeds)) { + embeds = [embeds]; + } + embeds.forEach(function (embed) { + self.errors.push(new HalError(embed)); + }); + } + }; + + return HalError; + }) + + +; + +'use strict'; + angular.module('hypermedia') /** diff --git a/src/halerror.js b/src/halerror.js index d972799..f15bb71 100644 --- a/src/halerror.js +++ b/src/halerror.js @@ -33,4 +33,4 @@ angular.module('hypermedia') }) -; \ No newline at end of file +; From c9666239fb0aca0e971b00c68e9d61d64cc8d9d5 Mon Sep 17 00:00:00 2001 From: Thomas Delnoij Date: Mon, 2 Nov 2015 15:19:57 +0100 Subject: [PATCH 5/6] Fix review comments --- dist/hypermedia.js | 75 +++++++++++----------- src/context.js | 3 +- src/context.spec.js | 10 +-- src/{halerror.js => vnderror.js} | 10 ++- src/{halerror.spec.js => vnderror.spec.js} | 10 +-- 5 files changed, 54 insertions(+), 54 deletions(-) rename src/{halerror.js => vnderror.js} (68%) rename src/{halerror.spec.js => vnderror.spec.js} (79%) diff --git a/dist/hypermedia.js b/dist/hypermedia.js index 2023bd9..3fcd8a8 100644 --- a/dist/hypermedia.js +++ b/dist/hypermedia.js @@ -337,7 +337,8 @@ angular.module('hypermedia') 'responseError': function (response) { var contentType = response.headers('Content-Type'); var handler = handlers[contentType]; - return handler ? handler(response) : $q.reject(response); + response.error = (handler ? handler(response) : {message: response.statusText}); + return $q.reject(response); }, 'registerErrorHandler': function (contentType, handler) { if (!handlers) handlers = {}; @@ -359,43 +360,6 @@ angular.module('hypermedia') 'use strict'; -angular.module('hypermedia') - - .run(function ($q, ResourceContext, HalError) { - var vndErrorHandler = function (response) { - response.error = new HalError(response.data); - - return $q.reject(response); - }; - - ResourceContext.registerErrorHandler('application/vnd+error', vndErrorHandler); - }) - - .factory('HalError', function () { - var HalError = function (data) { - var self = this; - this.message = data.message; - this.errors = []; - - var embeds = (data._embedded ? data._embedded['ilent:error'] : undefined); - if (embeds) { - if (!Array.isArray(embeds)) { - embeds = [embeds]; - } - embeds.forEach(function (embed) { - self.errors.push(new HalError(embed)); - }); - } - }; - - return HalError; - }) - - -; - -'use strict'; - angular.module('hypermedia') /** @@ -1045,3 +1009,38 @@ angular.module('hypermedia') }) ; + +'use strict'; + +angular.module('hypermedia') + + .run(function ($q, ResourceContext, VndError) { + var vndErrorHandler = function (response) { + return new VndError(response.data); + }; + + ResourceContext.registerErrorHandler('application/vnd+error', vndErrorHandler); + }) + + .factory('VndError', function () { + var HalError = function (data) { + var self = this; + this.message = data.message; + this.errors = []; + + var embeds = (data._embedded ? data._embedded.errors : undefined); + if (embeds) { + if (!Array.isArray(embeds)) { + embeds = [embeds]; + } + embeds.forEach(function (embed) { + self.errors.push(new HalError(embed)); + }); + } + }; + + return HalError; + }) + + +; diff --git a/src/context.js b/src/context.js index 97d2671..eb0e1c4 100644 --- a/src/context.js +++ b/src/context.js @@ -243,7 +243,8 @@ angular.module('hypermedia') 'responseError': function (response) { var contentType = response.headers('Content-Type'); var handler = handlers[contentType]; - return handler ? handler(response) : $q.reject(response); + response.error = (handler ? handler(response) : {message: response.statusText}); + return $q.reject(response); }, 'registerErrorHandler': function (contentType, handler) { if (!handlers) handlers = {}; diff --git a/src/context.spec.js b/src/context.spec.js index 00d02a3..ccabdd3 100644 --- a/src/context.spec.js +++ b/src/context.spec.js @@ -49,23 +49,25 @@ describe('ResourceContext', function () { expect(spy).toHaveBeenCalled(); }); - it('rejects response if no matching error handler', function () { + it('rejects response with error if no matching error handler', function () { + var statusText = 'Validation error'; var promiseResult = null; var spy = jasmine.createSpy('spy'); - ResourceContext.registerErrorHandler('application/json', spy); + ResourceContext.registerErrorHandler('application/json', spy); context.httpGet(resource).catch(function (result) { promiseResult = result; }); $httpBackend.expectGET(resource.$uri, {'Accept': 'application/json'}) - .respond(500, {}, {'Content-Type': problemJson}); + .respond(500, {}, {'Content-Type': problemJson}, statusText); $httpBackend.flush(); expect(spy).not.toHaveBeenCalled(); + expect(promiseResult.error.message).toBe(statusText); expect(promiseResult.status).toBe(500); }); - it('invokes default error handler for content type \'application/vnd+error\'', function () { + it('invokes default error handler for content type "application/vnd+error"', function () { var promiseResult; var msg = 'Validatie fout'; context.httpGet(resource).catch(function (result) { diff --git a/src/halerror.js b/src/vnderror.js similarity index 68% rename from src/halerror.js rename to src/vnderror.js index f15bb71..fb2a4f4 100644 --- a/src/halerror.js +++ b/src/vnderror.js @@ -2,23 +2,21 @@ angular.module('hypermedia') - .run(function ($q, ResourceContext, HalError) { + .run(function ($q, ResourceContext, VndError) { var vndErrorHandler = function (response) { - response.error = new HalError(response.data); - - return $q.reject(response); + return new VndError(response.data); }; ResourceContext.registerErrorHandler('application/vnd+error', vndErrorHandler); }) - .factory('HalError', function () { + .factory('VndError', function () { var HalError = function (data) { var self = this; this.message = data.message; this.errors = []; - var embeds = (data._embedded ? data._embedded['ilent:error'] : undefined); + var embeds = (data._embedded ? data._embedded.errors : undefined); if (embeds) { if (!Array.isArray(embeds)) { embeds = [embeds]; diff --git a/src/halerror.spec.js b/src/vnderror.spec.js similarity index 79% rename from src/halerror.spec.js rename to src/vnderror.spec.js index 04db57b..3250856 100644 --- a/src/halerror.spec.js +++ b/src/vnderror.spec.js @@ -3,10 +3,10 @@ describe('HalError', function () { beforeEach(module('hypermedia')); - var HalError; + var VndError; - beforeEach(inject(function (_HalError_) { - HalError = _HalError_; + beforeEach(inject(function (_VndError_) { + VndError = _VndError_; })); it('can be constructed with embedded errors', function () { @@ -14,14 +14,14 @@ describe('HalError', function () { 'message': 'Validatie fout', '_links': {'profile': {'href': 'http://nocarrier.co.uk/profiles/vnd.error/'}}, '_embedded': { - 'ilent:error': { + 'errors': { 'message': 'Invalide nummer', '_links': {'profile': {'href': 'http://nocarrier.co.uk/profiles/vnd.error/'}} } } }; - var error = new HalError(data); + var error = new VndError(data); expect(error.message).toBe('Validatie fout'); expect(error.errors.length).toBe(1); From e7c5fe5218a97ed7d65821f4d1ff8d288282d8a9 Mon Sep 17 00:00:00 2001 From: Thomas Delnoij Date: Mon, 2 Nov 2015 15:31:58 +0100 Subject: [PATCH 6/6] Add documentation --- src/context.js | 9 +++++++++ src/context.spec.js | 2 +- src/vnderror.js | 8 ++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/context.js b/src/context.js index eb0e1c4..aecfd54 100644 --- a/src/context.js +++ b/src/context.js @@ -237,6 +237,15 @@ angular.module('hypermedia') } }]) + /** + * @ngdoc service + * @name errorInterceptor + * @description + * + * Intercepts error from server and invokes error handler for content-type, + * or default error handler if none is found. Error with message is published + * on response under 'error' key. + */ .factory('errorInterceptor', function ($q) { var handlers; return { diff --git a/src/context.spec.js b/src/context.spec.js index ccabdd3..da746b4 100644 --- a/src/context.spec.js +++ b/src/context.spec.js @@ -37,7 +37,7 @@ describe('ResourceContext', function () { it('invokes error handler for content type', function () { var spy = jasmine.createSpy('spy').and.callFake(function (response) { - return $q.reject(response); + return {}; }); ResourceContext.registerErrorHandler(problemJson, spy); diff --git a/src/vnderror.js b/src/vnderror.js index fb2a4f4..3a75446 100644 --- a/src/vnderror.js +++ b/src/vnderror.js @@ -10,6 +10,14 @@ angular.module('hypermedia') ResourceContext.registerErrorHandler('application/vnd+error', vndErrorHandler); }) + /** + * @ngdoc type + * @name VndError + * @description + * + * VndError represents errors from server with content type 'application/vnd+error', + * see: https://github.com/blongden/vnd.error + */ .factory('VndError', function () { var HalError = function (data) { var self = this;