From 881e740bb9de26e9d62b9806560c833efddf397a Mon Sep 17 00:00:00 2001 From: Mike Gerwitz Date: Thu, 28 May 2015 12:16:02 -0400 Subject: [PATCH] JsonResponse returns parse error or combined request/parse on failure --- src/dapi/format/JsonResponse.js | 84 +++++++++++++++++++++------- test/dapi/format/JsonResponseTest.js | 57 +++++++++++++++---- 2 files changed, 109 insertions(+), 32 deletions(-) diff --git a/src/dapi/format/JsonResponse.js b/src/dapi/format/JsonResponse.js index 49ebe9e..b88ad9a 100644 --- a/src/dapi/format/JsonResponse.js +++ b/src/dapi/format/JsonResponse.js @@ -38,6 +38,10 @@ module.exports = Trait( 'JsonResponse' ) * property---`text`---containing the response text that failed to * parse. * + * If a request error occurs in conjunction with a parse error, then + * both errors will be returned in a single error object under the + * `list` property. + * * @param {string} data binary data to transmit * @param {function(?Error,*)} callback continuation upon reply * @@ -45,32 +49,70 @@ module.exports = Trait( 'JsonResponse' ) */ 'virtual abstract override public request': function( data, callback ) { + var _self = this; + this.__super( data, function( err, resp ) { - if ( err !== null ) - { - callback( err, resp ); - return; - } - - try - { - var data = JSON.parse( resp ); - } - catch ( e ) - { - // parsing failed; provide response text in addition to - // original data so that the caller can handle how they - // please - callback( e, { text: resp } ); - - return; - } - - callback( null, data ); + _self._tryParse( err, resp, callback ); } ); return this; + }, + + + /** + * Attempt to parse SRC as JSON and invoke callback according to the + * rules of `#request` + * + * @param {?Error} err response error + * @param {string} src JSON string + * @param {function(?Error,*)} callback continuation + * + * @return {undefined} + */ + 'private _tryParse': function( err, src, callback ) + { + try + { + var data = JSON.parse( src ); + } + catch ( e ) + { + // parsing failed; provide response text in addition to + // original data so that the caller can handle how they + // please + callback( + this._getReturnError( err, e ), + { text: src } + ); + + return; + } + + callback( err, data ); + }, + + + /** + * Produce the parse error, or a combined error containing both the + * original and parse errors + * + * @param {?Error} orig response error + * @param {Error} parse parse error + * + * @return {Error} parse error or combined error + */ + 'private _getReturnError': function( orig, parse ) + { + if ( !orig ) + { + return parse; + } + + var e = Error( "Multiple errors occurred; see `list` property" ); + e.list = [ orig, parse ]; + + return e; } } ); diff --git a/test/dapi/format/JsonResponseTest.js b/test/dapi/format/JsonResponseTest.js index 633a93f..f74ea50 100644 --- a/test/dapi/format/JsonResponseTest.js +++ b/test/dapi/format/JsonResponseTest.js @@ -83,19 +83,54 @@ describe( 'dapi.format.JsonRepsonse trait', function() } ); - it( 'proxy error from encapsulated DataApi', function( done ) + describe( 'on request error from supertype', function() { - var e = Error( 'foo' ), - chk = {}; + it( 'attempts to format output as JSON', function( done ) + { + var chk = '{"foo": "bar"}'; - _createStubbedDapi( e, chk ) - .request( '', function( err, data ) - { - // data should also be cleared out - expect( err ).to.equal( e ); - expect( data ).to.equal( chk ); - done(); - } ); + _createStubbedDapi( null, chk ) + .request( '', function( _, data ) + { + expect( data ).to.be.a( 'object' ); + expect( data.foo ).to.equal( "bar" ); + done(); + } ); + } ); + + + it( 'proxies error when JSON output valid', function( done ) + { + var e = Error( 'foo' ); + + _createStubbedDapi( e, '{}' ) + .request( '', function( err, _ ) + { + expect( err ).to.equal( e ); + done(); + } ); + } ); + + + it( 'produces both errors on bad JSON output', function( done ) + { + var e = Error( 'foo' ); + + _createStubbedDapi( e, 'BAD JSON' ) + .request( '', function( err, _ ) + { + // the main error should indicate both + expect( err ).to.be.instanceOf( Error ); + + // and we should provide references to both + expect( err.list[ 0 ] ).to.equal( e ); + expect( err.list[ 1 ] ).to.be.instanceOf( + SyntaxError + ); + + done(); + } ); + } ); } ); } ); } );