The middleware pipeline should halt on error

[dougwilson]

Right now, when next(err) is called, the pipeline will switch into an "error mode" and start to process the error down the error middleware. If some middleware then calls next() (i.e. double-next) then the normal pipeline processing will resume.

I think that once an err has been provided to the pipeline, a flag should be set in the pipeline and simply ignore all next() calls (non-error-pipeline calls). This would probably be a sane way to manage errors, as it would be an analogue to someone raising a flag when they get on('error', fn) called and want to ignore the results from non-cancel-able async operations.

[TrejGun]

i'm not agree with you. my argument is a then-catch flow from promises

new Promise(resolve => {
	setImmediate(() => {
		resolve();
	});
})
	.catch(() => {
		console.log("will NOT come here");
	})
	.then(() => {
		console.log("will come here 1");
		throw new Error();
	})
	.then(() => {
		console.log("will NOT come here");
	})
	.catch(e => {
		console.log("will come here 2");
		throw new e;
	})
	.catch(e => {
		console.log("will come here 3");
	})
	.catch(() => {
		console.log("will NOT come here");
	})
	.then(() => {
		console.log("will come here 4");
	});

[TrejGun]

I mean if you ever finish express5 which is going to support promises, this is what people will expect from its behavior

[dougwilson]

Hi @TrejGun good point. What is your idea for how to change the behavior to solve the double-next() issues?

[TrejGun]

i believe when you want to pass an error between error-middlewares you should call next(error) but not next()

app.use(function (err, req, res, next) {
  next(err);
})

if you call next() from error middleware

app.use(function (err, req, res, next) {
  next();
})

you should resume non-error-pipeline

[dougwilson]

Gotcha. So what I'm trying to brain storm is a solution to the following problem:

app.use(function (req, res, next) {
  setImmediate(function () {
    // some async activity fails
    next(new Error())
  })
  setImmediate(function () {
    // user has logic error that didn't stop the processing
    // so a second next() occurs thinking successful
    next()
  })
})

So the issue I'm trying to think of how to handle better is that second next() call, after an error occurs. Right now it breaks really odd, and of course is all silent about how that happens. It basically will re-enter into the middleware processing even after the res may have been written to because of the error handling, resulting in those annoying "can't send headers" errors at best, silent race conditions at worse.

What is your take on a solution? How do Promises handle the situation?

[TrejGun]

new Promise((resolve, reject) => {
	setImmediate(() => {
		reject(new Error());
	});
	setImmediate(() => {
		resolve();
	});
})
	.then(() => {
		console.log("then");
	})
	.catch(e => {
		console.log("catch", e);
	});

says

catch Error
    at Immediate.setImmediate (~/test.js:3:10)
    at runCallback (timers.js:781:20)
    at tryOnImmediate (timers.js:743:5)
    at processImmediate [as _immediateCallback] (timers.js:714:5)

so just skip second call completely but put a warning to console

Callback called more than once.