Type |聽Version
---|---
Improvement/Question | 1.4.15
As it's mentioned at NodeJS Docs about uncaughtExceptions:
Unhandled exceptions inherently mean that an application is in an undefined state. Attempting to resume application code without properly recovering from the exception can cause additional unforeseen and unpredictable issues.
Attempting to resume normally after an uncaught exception can be similar to pulling out of the power cord when upgrading a computer -- nine out of ten times nothing happens - but the 10th time, the system becomes corrupted.
The correct use of 'uncaughtException' is to perform synchronous cleanup of allocated resources (e.g. file descriptors, handles, etc) before shutting down the process. It is not safe to resume normal operation after 'uncaughtException'.
To restart a crashed application in a more reliable way, whether uncaught Exceptions is emitted or not, an external monitor should be employed in a separate process to detect application failures and recover or restart as needed.
So, we should not depend on On Error Resume Next mechanism. I noticed in the source code where, for example, an uncaught exception is handled with a try/catch and passed to next(err) and I have recently encountered an uncaught exception (And server has been still live after this exception) and found that some already existing URLs became 404 (I think this happens because the exception has been thrown on bootstrapping the server)
Therefore, I suggest leaving the server crashes normally on uncaught exceptions leaving the user the ability to restart it with any external monitor like nodemon to detect application failures and recover or restart as needed.
Hi @m0uneer,
I share your vision of management error, but isn't possible to do that and I'll explain why :)
I think, the try/catch you're talking about is the following:
https://github.com/Romakita/ts-express-decorators/blob/master/src/mvc/class/HandlerBuilder.ts#L125-L151
If you look this method, you'll see it use a async/await instruction. That mean, the method will be wrapped into a Promise by TypeScript. So the error will never throwing and caught by the Node vm.
This behavior is wanted because with Ts.ED you can do that:
async myMethod() {
throw new BadRequest("Error")
}
You also have the converter and validation that emits functional errors ;)
This is why you have a try/catch with a next(err). The error is forwarded to middlewares to let the developer to handle this error and apply the correct process (send a response or kill the process).
The GlobalErrorHandlerMiddleware is here for this reason. You can customize this middleware with @OverrideMiddleware() decorator.
Example:
@OverrideMiddleware()
export class CustomGlobalErrorHandlerMiddleware implements IMiddlewareError {
use(@Err() error: any,
@Request() request: Express.Request,
@Response() response: Express.Response): any {
const toHTML = (message = "") => message.replace(/\n/gi, "<br />");
if (error instanceof Exception || error.status) {
request.log.error({
error: {
message: error.message,
stack: error.stack,
status: error.status
}
});
response.status(error.status).send(toHTML(error.message));
return;
}
if (typeof error === "string") {
response.status(404).send(toHTML(error));
return;
}
request.log.error({
error: {
status: 500,
message: error.message,
stack: error.stack
}
});
response.status(error.status || 500).send("Internal Error");
// You can kill the process here
// but don't use the throw here.
return;
}
}
I hope, my explanations is clear and will help you.
If you have a best idea, you can propose a Pull Request or just reply in this read to talk about :)
See you @m0uneer
Romain
Most helpful comment
Hi @m0uneer,
I share your vision of management error, but isn't possible to do that and I'll explain why :)
I think, the try/catch you're talking about is the following:
https://github.com/Romakita/ts-express-decorators/blob/master/src/mvc/class/HandlerBuilder.ts#L125-L151
If you look this method, you'll see it use a async/await instruction. That mean, the method will be wrapped into a Promise by TypeScript. So the error will never throwing and caught by the Node vm.
This behavior is wanted because with Ts.ED you can do that:
This is why you have a try/catch with a next(err). The error is forwarded to middlewares to let the developer to handle this error and apply the correct process (send a response or kill the process).
The GlobalErrorHandlerMiddleware is here for this reason. You can customize this middleware with @OverrideMiddleware() decorator.
Example:
I hope, my explanations is clear and will help you.
If you have a best idea, you can propose a Pull Request or just reply in this read to talk about :)
See you @m0uneer
Romain