Skip to content

Commit

Permalink
PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonutEspresso
DonutEspresso committed Aug 16, 2017
1 parent e079e11 commit 869a41c
Showing 1 changed file with 72 additions and 53 deletions.
125 changes: 72 additions & 53 deletions lib/server.js
View file
Expand Up @@ -186,10 +186,11 @@ function ifError(n) {


/**
* TODO: this is relevant for CORS only. Should move this out eventually to a
* userland middleware?
* returns true if the router generated a 404 for an options request.
*
* returns true if an error generated is for an options request.
* TODO: this is relevant for CORS only. Should move this out eventually to a
* userland middleware? This also seems a little like overreach, as there is no
* option to opt out of this behavior today.
* @private
* @function optionsError
* @param {Object} err an error object
Expand All @@ -198,22 +199,23 @@ function ifError(n) {
* @returns {Boolean}
*/
function optionsError(err, req, res) {
if (err.statusCode === 404 && req.method === 'OPTIONS' && req.url === '*') {
return true;
}
return false;
return (
err.statusCode === 404 &&
req.method === 'OPTIONS' &&
req.url === '*'
);
}


/**
* map an Error's .name property into the actual event name that is emitted
* by the restify server object.
* @function
* @private getErrorName
* @private errEvtNameFromError
* @param {Object} err an error object
* @returns {String} an event name to emit
*/
function getErrorName(err) {
function errEvtNameFromError(err) {
if (err.name === 'ResourceNotFoundError') {
// remap the name for router errors
return 'NotFound';
Expand Down Expand Up @@ -819,17 +821,31 @@ Server.prototype._route = function _route(req, res, name, cb) {


/*
* `cb()` is called when execution is complete. "completion" can occur when:
* 1) all functions in handler chain have been executed
* 2) users invoke `next(false)`. this indicates the chain should stop
* executing immediately.
* 3) users invoke `next(err)`. this is sugar for calling res.send(err) and
* firing any error events. after error events are done firing, it will also
* stop execution.
*
* The goofy checks in next() are to make sure we fire the DTrace
* probes after an error might have been sent, as in a handler
* return next(new Error) is basically shorthand for sending an
* error via res.send(), so we do that before firing the dtrace
* probe (namely so the status codes get updated in the
* response).
*
* Callers can stop the chain from proceding if they do
* return next(false); This is useful for non-errors, but where
* a response was sent and you don't want the chain to keep
* going.
* there are two important closure variables in this function as a result of
* the way `next()` is currently implemented. `next()` assumes logic is sync,
* and automatically calls cb() when a route is considered complete. however,
* for case #3, emitted error events are async and serial. this means the
* automatic invocation of cb() cannot occur:
*
* 1) `emittedError` - this boolean is set to true when next is called with an
* error. this var is used to avoid automatic invocation of cb(), which is
* delayed until all async error events are fired.
* 2) `done` - when next is invoked with a value of `false`, or handler if
*
* @private
* @function _run
Expand All @@ -842,7 +858,6 @@ Server.prototype._route = function _route(req, res, name, cb) {
* @returns {undefined}
*/
Server.prototype._run = function _run(req, res, route, chain, cb) {
var d;
var i = -1;
var id = dtrace.nextId();
req._dtraceId = id;
Expand Down Expand Up @@ -881,15 +896,20 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
});

function next(arg) {
// default value of done determined by whether or not there is another
// function in the chain and/or if req was not already closed. we will
// consume the value of `done` after dealing with any passed in values
// of `arg`.
var done = false;

if (arg) {
if (typeof arg !== 'undefined') {
if (arg instanceof Error) {
// the emitting of the error events are async, so we can not
// complete this invocation of run() until it returns. setting
// the emittedError flag is flag that is returned sync -
// setting this flag will allow us to to not call cb()
// automatically later in this function.
// complete this invocation of run() until it returns. calling
// _emitErrorEvents is async, but returns a sync flag. when this
// flag is true, we will avoid automatically calling cb at the
// end of this function, which causes server to move on to the
// next handler in the chain.
emittedError = self._emitErrorEvents(req, res, route, arg,
function emitErrorsDone() {
res.send(arg);
Expand All @@ -909,7 +929,9 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
return (false);
}

// Stop running the rest of this route since we're redirecting
// Stop running the rest of this route since we're redirecting.
// do this instead of setting done since the route technically
// isn't complete yet.
return self._route(req, res, arg, function (r, ctx) {
req.context = req.params = ctx;
req.route = r.spec;
Expand All @@ -931,21 +953,18 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
}
self._run(req, res, r, _chain, cb);
});
} else if (arg === false) {
done = true;
}
}

if (arg === false) {
done = true;
}

// Fire DTrace done for the previous handler.
if ((i + 1) > 0 && chain[i] && !chain[i]._skip) {
req.endHandlerTimer(handlerName);
}

// Run the next handler up
if (!done && chain[++i] && !_reqClosed(req)) {

if (chain[i]._skip) {
return (next());
}
Expand All @@ -969,6 +988,8 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
return (chain[i].call(self, req, res, n));
}

// if we have reached this last section of next(), then we are 'done'
// with this route.
dtrace._rstfy_probes['route-done'].fire(function () {
return ([
self.name,
Expand All @@ -979,6 +1000,8 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
]);
});

// if there is no route, it's because this is running the `pre` handler
// chain.
if (route === null) {
self.emit('preDone', req, res);
} else {
Expand All @@ -987,8 +1010,9 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
self.emit('done', req, res, route);
}

// Don't return cb here if we emit an error since we will cb after the
// handler fires.
// Don't return cb here if errors are currently being emitted. they are
// async, so we cannot fire cb until all error events are done
// emitting.
if (!emittedError) {
return (cb ? cb(arg) : true);
} else {
Expand All @@ -998,11 +1022,6 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {

var n1 = once(next);

// support ifError only if domains are on
if (self.handleUncaughtExceptions === true) {
n1.ifError = ifError(n1);
}

dtrace._rstfy_probes['route-start'].fire(function () {
return ([
self.name,
Expand All @@ -1017,29 +1036,29 @@ Server.prototype._run = function _run(req, res, route, chain, cb) {
req.timers = [];

if (!self.handleUncaughtExceptions) {
n1();
return;
return n1();
} else {
n1.ifError = ifError(n1);
// Add the uncaughtException error handler.
var d = domain.create();
d.add(req);
d.add(res);
d.on('error', function onError(err) {
if (err._restify_next) {
err._restify_next(err);
} else {
log.trace({err: err}, 'uncaughtException');
self.emit('uncaughtException', req, res, route, err);
self._finishReqResCycle(req, res, route, err);
}
});
return d.run(n1);
}

// Add the uncaughtException error handler.
d = domain.create();
d.add(req);
d.add(res);
d.on('error', function onError(err) {
if (err._restify_next) {
err._restify_next(err);
} else {
log.trace({err: err}, 'uncaughtException');
self.emit('uncaughtException', req, res, route, err);
self._finishReqResCycle(req, res, route, err);
}
});
d.run(n1);
};


/**
* set up the request by before routing and executing handler chain.
* set up the request before routing and execution of handler chain functions.
* @private
* @function _setupRequest
* @param {Object} req the request object
Expand Down Expand Up @@ -1070,7 +1089,7 @@ Server.prototype._setupRequest = function _setupRequest(req, res) {
* emit error events when errors are encountered either while attempting to
* route the request (via router) or while executing the handler chain.
* @private
* @function emitErrorEvents
* @function _emitErrorEvents
* @param {Object} req the request object
* @param {Object} res the response object
* @param {Object} route the current route, if applicable
Expand All @@ -1083,7 +1102,7 @@ Server.prototype._emitErrorEvents =
function _emitErrorEvents(req, res, route, err, cb) {

var self = this;
var errName = getErrorName(err);
var errName = errEvtNameFromError(err);

req.log.trace({
err: err,
Expand All @@ -1110,7 +1129,7 @@ function _emitErrorEvents(req, res, route, err, cb) {
self.emit(errEvtName, req, res, err, function emitErrDone() {
// the error listener may return arbitrary objects, throw
// them away and continue on. don't want vasync to take
// that error and stop, we want to emit every single event..
// that error and stop, we want to emit every single event.
return vasyncCb();
});
}
Expand Down

0 comments on commit 869a41c

Please sign in to comment.