From 04314ed9eef0974100cd193c8a5e247deb0d4bbd Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Tue, 16 Jun 2015 11:26:33 -0700 Subject: [PATCH] Add support for declaring status code defaults --- lib/http-context.js | 51 ++++++++++++++++++++++-- lib/rest-adapter.js | 4 +- lib/shared-method.js | 34 +++++++++++++--- test/authorize-hook.test.js | 1 - test/rest.test.js | 78 ++++++++++++++++++++++++++++++++----- 5 files changed, 149 insertions(+), 19 deletions(-) diff --git a/lib/http-context.js b/lib/http-context.js index 5f7f962..6a311ec 100644 --- a/lib/http-context.js +++ b/lib/http-context.js @@ -38,6 +38,7 @@ function HttpContext(req, res, method, options) { this.args = this.buildArgs(method); this.methodString = method.stringName; this.supportedTypes = this.options.supportedTypes || DEFAULT_SUPPORTED_TYPES; + this.result = {}; if (this.supportedTypes === DEFAULT_SUPPORTED_TYPES && !this.options.xml) { // Disable all XML-based types by default @@ -304,6 +305,9 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) { var pipe = http && http.pipe; var pipeDest = pipe && pipe.dest; var pipeSrc = pipe && pipe.source; + var ctx = this; + var defaultErrorStatus = http && http.errorStatus; + var res = this.res; if (pipeDest) { // only support response for now @@ -313,7 +317,7 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) { this.res.header('Content-Type', 'application/json'); this.res.header('Transfer-Encoding', 'chunked'); - var stream = method.invoke(scope, args, this.options, fn); + var stream = method.invoke(scope, args, this.options, ctx, fn); stream.pipe(this.res); break; default: @@ -324,7 +328,7 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) { // only support request for now switch (pipeDest) { case 'req': - this.req.pipe(method.invoke(scope, args, this.options, fn)); + this.req.pipe(method.invoke(scope, args, this.options, ctx, fn)); break; default: fn(new Error('unsupported pipe source')); @@ -332,7 +336,43 @@ HttpContext.prototype.invoke = function(scope, method, fn, isCtor) { } } else { // simple invoke - method.invoke(scope, args, this.options, fn); + method.invoke(scope, args, this.options, ctx, function(err, result) { + if (err) { + if (defaultErrorStatus) { + if(!(err.status || err.statusCode)) { + res.status(defaultErrorStatus); + } + } + return fn(err); + } + fn(null, result); + }); + } +}; + +HttpContext.prototype.setReturnArgByName = function(name, value) { + var returnDesc = this.method.getReturnArgDescByName(name); + var result = this.result; + var res = this.res; + + if (!returnDesc) { + return debug('warning: cannot set return value for arg' + + ' (%s) without description!', name); + } + + if (returnDesc.root) { + this.result = value; + } else if (returnDesc.http) { + switch (returnDesc.http.target) { + case 'status': + res.status(value); + break; + case 'header': + res.set(returnDesc.header || name, value); + break; + } + } else { + result[name] = value; } }; @@ -384,6 +424,11 @@ HttpContext.prototype.done = function() { var data = this.result; var res = this.res; var accepts = this.req.accepts(this.supportedTypes); + var defaultStatus = this.method.http.status; + + if (defaultStatus) { + res.status(defaultStatus); + } if (this.req.query._format) { accepts = this.req.query._format.toLowerCase(); diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index 02391df..77cb0b6 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -359,7 +359,9 @@ RestAdapter.errorHandler = function(options) { err.status = err.statusCode = 500; } - res.statusCode = err.statusCode || err.status || 500; + if (res.statusCode === undefined || res.statusCode < 400) { + res.statusCode = err.statusCode || err.status || 500; + } debug('Error in %s %s: %s', req.method, req.url, err.stack); var data = { diff --git a/lib/shared-method.js b/lib/shared-method.js index 29f43df..2401885 100644 --- a/lib/shared-method.js +++ b/lib/shared-method.js @@ -24,6 +24,9 @@ var assert = require('assert'); * @param {Boolean} [options.isStatic] Is the method a static method or a * a `prototype` method * @param {Array} [options.aliases] A list of aliases for the + * @param {Array} [options.http] HTTP only options + * @param {Number} [options.http.status] The default status code + * @param {Number} [options.http.errorStatus] The default error status code * `sharedMethod.name` * @param {Array|Object} [options.accepts] An `Array` of argument definitions * that describe the arguments of the `SharedMethod`. @@ -144,6 +147,17 @@ SharedMethod.fromFunction = function(fn, name, sharedClass, isStatic) { }); }; +SharedMethod.prototype.getReturnArgDescByName = function(name) { + var returns = this.returns; + var desc; + for (var i = 0; i < returns.length; i++) { + desc = returns[i]; + if (desc && ((desc.arg || desc.name) === name)) { + return desc; + } + } +}; + /** * Execute the remote method using the given arg data. * @@ -153,7 +167,7 @@ SharedMethod.fromFunction = function(fn, name, sharedClass, isStatic) { * @param {Function} cb callback `fn(err, result)` containing named result data */ -SharedMethod.prototype.invoke = function(scope, args, remotingOptions, cb) { +SharedMethod.prototype.invoke = function(scope, args, remotingOptions, ctx, cb) { var accepts = this.accepts; var returns = this.returns; var errors = this.errors; @@ -161,6 +175,11 @@ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, cb) { var sharedMethod = this; var formattedArgs = []; + if (typeof ctx === 'function') { + cb = ctx; + ctx = undefined; + } + if (cb === undefined && typeof remotingOptions === 'function') { cb = remotingOptions; remotingOptions = {}; @@ -189,8 +208,9 @@ SharedMethod.prototype.invoke = function(scope, args, remotingOptions, cb) { if (err) { return cb(err); } - - var result = SharedMethod.toResult(returns, [].slice.call(arguments, 1)); + // args without err + var rawArgs = [].slice.call(arguments, 1); + var result = SharedMethod.toResult(returns, rawArgs, ctx); debug('- %s - result %j', sharedMethod.name, result); @@ -457,7 +477,7 @@ SharedMethod.convertArg = function(accept, raw) { * results from a remoting function, based on `returns`. */ -SharedMethod.toResult = function(returns, raw) { +SharedMethod.toResult = function(returns, raw, ctx) { var result = {}; if (!returns.length) { @@ -471,6 +491,7 @@ SharedMethod.toResult = function(returns, raw) { if (item.root) { result = convert(raw[index]); + if (ctx) ctx.setReturnArgByName(item.name || item.arg, raw[index]); return false; } @@ -478,7 +499,10 @@ SharedMethod.toResult = function(returns, raw) { }); returns.forEach(function(item, index) { - result[item.name || item.arg] = convert(raw[index]); + var value = convert(raw[index]); + var name = item.name || item.arg; + result[name] = value; + if (ctx) ctx.setReturnArgByName(name, value); }); return result; diff --git a/test/authorize-hook.test.js b/test/authorize-hook.test.js index bb87a4b..043aa70 100644 --- a/test/authorize-hook.test.js +++ b/test/authorize-hook.test.js @@ -55,4 +55,3 @@ describe('authorization hook', function() { remotes.invoke(method, args, callback); } }); - diff --git a/test/rest.test.js b/test/rest.test.js index 4674ec4..0927b2a 100644 --- a/test/rest.test.js +++ b/test/rest.test.js @@ -1753,17 +1753,77 @@ describe('strong-remoting-rest', function() { }); }); - it('returns 404 for unknown method of a shared class', function(done) { - var classUrl = givenSharedStaticMethod().classUrl; + describe('status codes', function() { + describe('using a custom satus code', function() { + it('returns a custom status code', function(done) { + var method = givenSharedStaticMethod( + function fn(cb) { + cb(); + }, + { + http: { status: 201 } + } + ); + json(method.url) + .expect(201, done); + }); + it('returns a custom error status code', function(done) { + var method = givenSharedStaticMethod( + function fn(cb) { + cb(new Error('test error')); + }, + { + http: { status: 201, errorStatus: 508 } + } + ); + json(method.url) + .expect(508, done); + }); + it('returns a custom error status code (using the err object)', function(done) { + var method = givenSharedStaticMethod( + function fn(cb) { + var err = new Error('test error'); + err.status = 555; + cb(err); + }, + { + http: { status: 201, errorStatus: 508 } + } + ); + json(method.url) + .expect(555, done); + }); + it('returns a custom status code from a callback arg', function(done) { + var exampleStatus = 222; + var method = givenSharedStaticMethod( + function fn(status, cb) { + cb(null, status); + }, + { + accepts: { arg: 'status', type: 'number' }, + returns: { + arg: 'status', + http: { target: 'status' } + } + } + ); + json(method.url + '?status=' + exampleStatus) + .expect(exampleStatus, done); + }); + }); - json(classUrl + '/unknown-method') - .expect(404, done); - }); + it('returns 404 for unknown method of a shared class', function(done) { + var classUrl = givenSharedStaticMethod().classUrl; - it('returns 404 with standard JSON body for uknown URL', function(done) { - json('/unknown-url') - .expect(404) - .end(expectErrorResponseContaining({status: 404}, done)); + json(classUrl + '/unknown-method') + .expect(404, done); + }); + + it('returns 404 with standard JSON body for uknown URL', function(done) { + json('/unknown-url') + .expect(404) + .end(expectErrorResponseContaining({status: 404}, done)); + }); }); it('returns correct error response body', function(done) {