Revert async formatters

lib/response.js

@@ -96,66 +96,6 @@ Response.prototype.charSet = function charSet(type) {
 };
 
 
-/**
- * finds a formatter that is both acceptable and works for the content-type
- * specified on the response. Can return two errors:
- *      NotAcceptableError - couldn't find a suitable formatter
- *      InternalServerError - couldn't find a fallback formatter
- * @public
- * @function _findFormatter
- * @param    {Function} callback a callback fn
- * @returns  {undefined}
- */
-Response.prototype._findFormatter = function _findFormatter(callback) {
-
-    var formatter;
-    var type = this.contentType || this.getHeader('Content-Type');
-
-    if (!type) {
-        if (this.req.accepts(this.acceptable)) {
-            type = this.req.accepts(this.acceptable);
-        }
-
-        if (!type) {
-            return callback(new errors.NotAcceptableError({
-                message: 'could not find suitable formatter'
-            }));
-        }
-    } else if (type.indexOf(';') !== '-1') {
-        type = type.split(';')[0];
-    }
-
-    if (!(formatter = this.formatters[type])) {
-
-        if (type.indexOf('/') === -1) {
-            type = mime.lookup(type);
-        }
-
-        if (this.acceptable.indexOf(type) === -1) {
-            type = 'application/octet-stream';
-        }
-
-        formatter = this.formatters[type] || this.formatters['*/*'];
-
-        // this is a catastrophic case - should always fall back on
-        // octet-stream but if for some reason that's gone, return a 500.
-        if (!formatter) {
-            return callback(new errors.InternalServerError({
-                message: 'could not find formatter for application/octet-stream'
-            }));
-        }
-    }
-
-    if (this._charSet) {
-        type = type + '; charset=' + this._charSet;
-    }
-
-    this.setHeader('Content-Type', type);
-
-    return callback(null, formatter, type);
-};
-
-
 /**
  * retrieves a header off the response.
  * @public
@@ -269,13 +209,11 @@ Response.prototype.link = function link(l, rel) {
  * @param    {Number} code http status code
  * @param    {Object | Buffer | Error} body the content to send
  * @param    {Object} headers  any add'l headers to set
- * @param    {Function} callback a callback for use with async formatters
- * @returns  {Object | undefined} when async formatter in use, returns null,
- * otherwise returns the response object
+ * @returns  {Object} the response object
  */
-Response.prototype.send = function send(code, body, headers, callback) {
+Response.prototype.send = function send(code, body, headers) {
     var self = this;
-    return self.__send(code, body, headers, callback, true);
+    return self.__send(code, body, headers, true);
 };
 
 
@@ -287,111 +225,129 @@ Response.prototype.send = function send(code, body, headers, callback) {
  * @param    {Number} code http status code
  * @param    {Object | Buffer | Error} body the content to send
  * @param    {Object} headers  any add'l headers to set
- * @param    {Function} callback a callback for use with async formatters
- * @returns  {Object | undefined} when async formatter in use, returns null,
- * otherwise returns the response object
+ * @returns  {Object} the response object
  */
-Response.prototype.sendRaw = function sendRaw(code, body, headers, callback) {
+Response.prototype.sendRaw = function sendRaw(code, body, headers) {
     var self = this;
-    return self.__send(code, body, headers, callback, false);
+    return self.__send(code, body, headers, false);
 };
 
 
 /**
  * internal implementation of send. convenience method that handles:
  * writeHead(), write(), end().
+ *
+ * Both body and headers are optional, but you MUST provide body if you are
+ * providing headers
+ *
  * @private
  * @private
- * @param    {Number} [maybeCode] http status code
- * @param    {Object | Buffer | Error} [maybeBody] the content to send
- * @param    {Object} [maybeHeaders] any add'l headers to set
- * @param    {Function} [maybeCallback] optional callback for async formatters
- * @param    {Function} format when false, skip formatting
+ * @param    {Number} [code] http status code
+ * @param    {Object | Buffer | String | Error} [body] the content to send
+ * @param    {Object} [headers] any add'l headers to set
+ * @param    {Boolean} [format] When false, skip formatting
  * @returns  {Object} returns the response object
  */
-Response.prototype.__send =
-function __send(maybeCode, maybeBody, maybeHeaders, maybeCallback, format) {
-
+Response.prototype.__send = function __send() {
     var self = this;
     var isHead = (self.req.method === 'HEAD');
     var log = self.log;
-    var code;
-    var body;
-    var headers;
-    var callback;
-
-    // normalize variadic args.
-    if (typeof maybeCode === 'number') {
-        // if status code was passed in, then signature should look like jsdoc
-        // signature and we only need to figure out headers/callback variation.
-        code = maybeCode;
-
-        // signature should look like jsdoc signature
-        body = maybeBody;
-
-        if (typeof maybeHeaders === 'function') {
-            callback = maybeHeaders;
-        } else {
-            callback = maybeCallback;
-            headers = maybeHeaders;
-        }
-    } else {
-        // if status code was omitted, then first arg must be body.
-        body = maybeCode;
+    var code, body, headers, format;
 
-        // now figure out headers/callback variation
-        if (typeof maybeBody === 'function') {
-            callback = maybeBody;
-        } else {
-            callback = maybeHeaders;
-            headers = maybeBody;
-        }
+    // derive arguments from types, one by one
+    var index = 0;
+    // Check to see if the first argument is a status code
+    if (typeof arguments[index] === 'number') {
+        code = arguments[index++];
+    }
+
+    // Check to see if the next argument is a body
+    if (typeof arguments[index] === 'object' ||
+        typeof arguments[index] === 'string') {
+        body = arguments[index++];
+    }
+
+    // Check to see if the next argument is a collection of headers
+    if (typeof arguments[index] === 'object') {
+        headers = arguments[index++];
     }
 
-    // if an error object is being sent and a status code isn't explicitly set,
-    // infer one from the error object itself.
+    // Check to see if the next argument is the format boolean
+    if (typeof arguments[index] === 'boolean') {
+        format = arguments[index++];
+    }
+
+    // Ensure the function was provided with arguments of the proper types,
+    // if we reach this line and there are still arguments, either one of the
+    // optional arguments was of an invalid type or we were provided with
+    // too many arguments
+    assert(arguments[index] === undefined,
+      'Unknown argument: ' + arguments[index] + '\nProvided: ' + arguments);
+
+    // Now lets try to derive values for optional arguments that we were not
+    // provided, otherwise we choose sane defaults.
+
+    // If the body is an error object and we were not given a status code, try
+    // to derive it from the error object, otherwise default to 500
     if (!code && body instanceof Error) {
-        self.statusCode = body.statusCode || 500;
-    } else {
-        self.statusCode = code || 200;
+        code = body.statusCode || 500;
     }
 
+    // Set sane defaults for optional arguments if they were not provided and
+    // we failed to derive their values
+    code = code || 200;
     headers = headers || {};
 
+    // Populate our response object with the derived arguments
+    self.statusCode = code;
+    self._body = body;
+    Object.keys(headers).forEach(function (k) {
+        self.setHeader(k, headers[k]);
+    });
+
+    // If log level is set to trace, output our constructed response object
     if (log.trace()) {
         var _props = {
             code: self.statusCode,
-            headers: headers
+            headers: self._headers
         };
 
         if (body instanceof Error) {
-            _props.err = body;
+            _props.err = self._body;
         } else {
-            _props.body = body;
+            _props.body = self._body;
         }
         log.trace(_props, 'response::send entered');
     }
 
-    self._body = body;
-
+    // Flush takes our constructed response object and sends it to the client
     function _flush(formattedBody) {
         self._data = formattedBody;
 
-        Object.keys(headers).forEach(function (k) {
-            self.setHeader(k, headers[k]);
-        });
-
+        // Flush headers
         self.writeHead(self.statusCode);
 
-        if (self._data && !(isHead || code === 204 || code === 304)) {
+        // Send body if it was provided
+        if (self._data) {
             self.write(self._data);
         }
 
+        // Finish request
         self.end();
 
+        // If log level is set to trace, log the entire response object
         if (log.trace()) {
             log.trace({res: self}, 'response sent');
         }
+
+        // Return the response object back out to the caller of __send
+        return self;
+    }
+
+    // 204 = No Content and 304 = Not Modified, we don't want to send the
+    // body in these cases. HEAD never provides a body.
+    if (isHead || code === 204 || code === 304) {
+        return _flush();
     }
 
     // if no formatting, assert that the value to be written is a string
@@ -404,68 +360,75 @@ function __send(maybeCode, maybeBody, maybeHeaders, maybeCallback, format) {
 
     // if no body, then no need to format. if this was an error caught by a
     // domain, don't send the domain error either.
-    if (body === null ||
-        body === undefined ||
-        (body instanceof Error && body.domain)) {
+    if (body === undefined || (body instanceof Error && body.domain)) {
         return _flush();
     }
 
-    // otherwise, try to find a formatter
-    self._findFormatter(
-    function foundFormatter(findErr, formatter, contentType) {
-
-        // handle missing formatters
-        if (findErr) {
-            // if user set a status code outside of the 2xx range, it probably
-            // outweighs being unable to format the response. set a status code
-            // then flush empty response.
-            if (self.statusCode >= 200 && self.statusCode < 300) {
-                self.statusCode = findErr.statusCode;
-            }
-            log.warn({
-                req: self.req,
-                err: findErr
-            }, 'error retrieving formatter');
-            return _flush();
+    // At this point we know we have a body that needs to be formatted, so lets
+    // derive the formatter based on the response object's properties
+
+    // _formatterError is used to handle any case where we were unable to
+    // properly format the provided body
+    function _formatterError(err) {
+        // If the user provided a non-success error code, we don't want to mess
+        // with it since their error is probably more important than our
+        // inability to format their message.
+        if (self.statusCode >= 200 && self.statusCode < 300) {
+            self.statusCode = err.statusCode;
         }
 
-        // if we have formatter, happy path.
-        var asyncFormat = (formatter && formatter.length === 4) ? true : false;
+        log.warn({
+            req: self.req,
+            err: err
+        }, 'error retrieving formatter');
 
-        if (asyncFormat === true) {
+        return _flush();
+    }
 
-            assert.func(callback, 'async formatter for ' + contentType +
-                                  ' requires callback to res.send()');
+    var formatter;
+    var type = self.contentType || self.getHeader('Content-Type');
 
-            // if async formatter error, propagate error back up to
-            // res.send() caller, most likely a handler.
-            return formatter.call(self, self.req, self, body,
-            function _formatDone(formatErr, formattedBody) {
+    // Check to see if we can find a valid formatter
+    if (!type && !self.req.accepts(self.acceptable)) {
+        return _formatterError(new errors.NotAcceptableError({
+            message: 'could not find suitable formatter'
+        }));
+    }
 
-                if (formatErr) {
+    // Derive type if not provided by the user
+    if (!type) {
+        type = self.req.accepts(self.acceptable);
+    }
 
-                    return callback(new errors.FormatterError(formatErr, {
-                        message: 'unable to format response for ' +
-                                    self.header('content-type'),
-                        context: {
-                            rawBody: body
-                        }
-                    }));
-                }
-                return _flush(formattedBody);
-            });
-        }
-        // for sync formatters, invoke formatter and send response body.
-        else {
-            _flush(formatter.call(self, self.req, self, body), body);
-            // users can always opt to pass in next, even when formatter is not
-            // async. invoke callback in that case. return null when no
-            // callback because eslint wants consistent-return.
-            return (callback) ? callback() : null;
-        }
-    });
+    type = type.split(';')[0];
+
+    if (!self.formatters[type] && type.indexOf('/') === -1) {
+        type = mime.lookup(type);
+    }
+
+    if (!self.formatters[type] && self.acceptable.indexOf(type) === -1) {
+        type = 'application/octet-stream';
+    }
+
+    formatter = self.formatters[type] || self.formatters['*/*'];
+
+    // If after the above attempts we were still unable to derive a formatter,
+    // provide a meaningful error message
+    if (!formatter) {
+        return _formatterError(new errors.InternalServerError({
+            message: 'could not find formatter for application/octet-stream'
+        }));
+    }
+
+    if (self._charSet) {
+        type = type + '; charset=' + self._charSet;
+    }
+
+    // Update header to the derived content type for our formatter
+    self.setHeader('Content-Type', type);
 
-    return self;
+    // Finally, invoke the formatter and flush the request with it's results
+    return _flush(formatter(self.req, self, body));
 };