Skip to content

Commit

Permalink
Merge pull request #5 from Vincit/req-path-in-error-messages
Browse files Browse the repository at this point in the history
Add requested path to router error messages
  • Loading branch information
elhigu committed Apr 20, 2017
2 parents b0435d7 + 3f6fd99 commit 89c236f
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 9 deletions.
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
"express": "^4.12.0"
},
"scripts": {
"test": "node_modules/.bin/istanbul --config=.istanbul.yml cover _mocha -- --slow 10 --timeout 5000 --reporter spec --recursive ./**/*.spec.js",
"test-no-coverage": "mocha --slow 10 --timeout 5000 --reporter spec --recursive --bail ./**/*.spec.js",
"test": "node_modules/.bin/istanbul --config=.istanbul.yml cover _mocha -- --slow 10 --timeout 5000 --reporter spec --recursive './**/*.spec.js'",
"test-no-coverage": "mocha --slow 10 --timeout 5000 --reporter spec --recursive --bail './**/*.spec.js'",
"coveralls": "cat ./test-coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js"
}
}
12 changes: 7 additions & 5 deletions route/router/Route.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Route.prototype.public = function () {

/**
* This handler sends response by itself and we shouldn't apply default response sending actions for this one.
*
*
* Handler must return promise, which doesn't resolve until res.send is called.
*
* @returns {Route}
Expand Down Expand Up @@ -148,7 +148,8 @@ Route.prototype.handlerMiddleware_ = function (req, res, next) {
return promise.then(function (result) {
if (result === NO_RESULT) {
if (!res.headersSent) {
throw new Error("Handler function did not return promise which won't resolve until response has been sent.");
throw new Error("When using .customResponse() handler, the promise returned must not resolve before the response has been sent. " +
"Requested path: " + req.path);
}
return;
}
Expand All @@ -158,7 +159,8 @@ Route.prototype.handlerMiddleware_ = function (req, res, next) {
sendResult(result, req, res);

This comment has been minimized.

Copy link
@elhigu

elhigu Apr 20, 2017

Author

here is the code sendResults, which always calls .send or throws an error...

}
if (!res.headersSent) {
throw new Error("Unexpected error, response was not sent by any handler for some reason. This should not be possible.");

This comment has been minimized.

Copy link
@elhigu

elhigu Apr 20, 2017

Author

Uh, actually this error message was fixed wrong. Will do something more descriptive and try to add test case. This codepath really should never run because case where no handler sends the response is handled earlier... Anyways looks like it is ran anyways in some rare cases where connection is closed by client but response is not sent either... or something like that.

throw new Error("Unexpected error, response was not sent by any handler for some reason. " +
"Requested path: " + req.path);
}
}).catch(next);
};
Expand All @@ -182,7 +184,7 @@ Route.prototype.handle_ = function (req, res, next) {
if (this.defaultAuthHandler) {
authHandlers.unshift(this.defaultAuthHandler);
} else {
throw new Error("No defaultAuthHandler set for non-public route.");
throw new Error("No defaultAuthHandler set for non-public route. Requested path: " + req.path);
}
}

Expand All @@ -197,7 +199,7 @@ Route.prototype.handle_ = function (req, res, next) {
} else if (ret instanceof Error) {
throw ret;
} else {
throw new Error("Invalid return value from auth handler.");
throw new Error("Invalid return value from auth handler. Requested path: " + req.path);
}
});
});
Expand Down
4 changes: 2 additions & 2 deletions route/tests/Router.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ describe('Router', function () {

var nextSpy = spy(function (err) {
expect(err instanceof Error).to.equal(true);
expect(err.message).to.equal("No defaultAuthHandler set for non-public route.");
expect(err.message).to.contain("No defaultAuthHandler set for non-public route.");
// Should not have called response.end or response.send or the handler.
expect(endSpy.calls).to.have.length(0);
expect(sendSpy.calls).to.have.length(0);
Expand Down Expand Up @@ -574,7 +574,7 @@ describe('Router', function () {
});

var nextSpy = spy(function (err) {
expect(err.message).to.contain('Handler function did not return promise');
expect(err.message).to.contain('When using .customResponse() handler, the promise returned must not resolve before');
});

return mockExpressRouter.simulateRequest(request, response, nextSpy)
Expand Down

0 comments on commit 89c236f

Please sign in to comment.