Express: next callback swallows falsy values

[despairblue]

The next() callback does not call the error handler for any falsy value.

See this example.

In the case of next(undefined) and throw undefined I see that this is cumbersome (in the former case even impossible, but in the latter case a flag could be set where the exception is caught).

The other cases could be handled by explicitly comparing to undefined.

The reason I'd like to see this is that passing something to next() that is not undefined is almost always something one wants to handle. If it comes from a third party library the developer might not even know that it could return something falsy. Or a third party library could throw something falsy and that is something one wants to know. At the moment express swallows this and responds with a 404. Stumbling onto this issue could take some time to debug.

I know that throwing 0, '', false etc. is something that should not be done, but the fact is that everyone who relies on third party code (without completely reviewing it) will eventually hit this case.

I'm also aware that this would be a breaking change, I just wanted to raise the issue and start a discussion.

Cheers

[dougwilson]

Unfortunately, I hear your concern, but we are not going to change it. The paradigm for checking for truthy values come from the Node.js-isms, and we align with the community on what to do. Many people still think they can use false for the error parameter, since everything ignores it (and it feels like saying "there is no error"). Also, many people use null, because they feel like it is the negative of new Error().

Just check out Node.js core itself: https://github.com/nodejs/node/search?utf8=%E2%9C%93&q=%22if+%28err%29%22

If you can convince Node.js core to change their checking, we will follow, but as long as this is the standard pattern, it won't be changing, I'm sorry.

[despairblue]

@dougwilson I understand. Thanks for the fast response! 👍

[dougwilson]

No problem! After I posted this, I went to go look to see if Node.js actually documented this callback pattern anywhere (since that is the origination for most uses of it within Node.js) and I found this: https://nodejs.org/api/errors.html#errors_error_propagation_and_interception

Besides all the examples there also just using if (err) { ... } to handle them, I actually found one place where they kind of tried to word the handling pattern:

Most asynchronous methods that accept a callback function will accept an Error object passed as the first argument to that function. If that first argument is not null and is an instance of Error, then an error occurred that should be handled.

I haven't even bothered to look at when this document was added to the source, and I don't think anyone has sat down to think about really fleshing it out well, but at least in the little said is similar to the "truthy check" in the examples & elsewhere in Node.js, except it technically lets a lot more things split past than the truthy check does, haha.

I wonder from this investigation if it is worthwhile to get Node.js core to flush out more specifics to reference (even if it's just outlining how the pattern has turned out organically ("truthy check").