ch-a-os / DocSort

Digitize and access everything, everywhere.
GNU General Public License v3.0
0 stars 1 forks source link

+ Error handler implemented #137

Closed Mondei1 closed 5 years ago

Mondei1 commented 5 years ago

Closes #78

Mondei1 commented 5 years ago

With this method my ApplicationError looks like this:

export class ApplicationError extends Error {
    constructor(message, status, request: Response) {
        super();

        Error.captureStackTrace(this, this.constructor);

        this.name = this.constructor.name;
        this.message = message || 'Something went wrong but we can\'t tell what. Please try again.';
        status = status || 500;

        // Send user error
        request.status(status);
        request.send({
            name: this.name,
            message: this.message,
            status: status
        });
    }
}

And the way you trigger it is that:

throw new ApplicationError('We cannot proceed your login request because some values are missing!', 401, res);

@720degreeLotus Do you think this is okay that way?

The old way was it to write:

errorHandler(err);
res.status(500).send();
Mondei1 commented 5 years ago

Now I added default error messages like A requested resource couldn't be found: % for the NotFoundError and priorities. Means If a NotFoundError or AuthenticationError occurs there is no big error message. Just a small warning in the console with a custom text or this default text.

ghost commented 5 years ago

After seeing the code and thinking about it, i would suggest we maybe try to follow this logic:

We also have, in sum, 3 ways of catching/throwing errors/warnings:

  1. uncaughtException (https://nodejs.org/api/process.html#process_event_uncaughtexception)
  2. unhandledRejection (https://nodejs.org/api/process.html#process_event_unhandledrejection)
  3. warning (https://nodejs.org/api/process.html#process_event_warning)

This way we could throw our own errors, and format them properly, but also catch every uncaught error and log it properly in our system, so we can analyse it and solve the issue.

@Mondei1 would be glad to hear your opinion on this :) This would save us a lot of code where we "if" "try" or "format" errors. This way we can just throw them and the rest will be done in the process.on-function.

I can try to change the code like i have described here and we can see if that works for us? I think i can do this within the next 1-2 days for sure =)

Also i just tested: Code is NOT executed after throwing an error when using process.on (this is correct, just wanted to make sure):

process.on('uncaughtException', function (err) {
  console.log(" -> error: " + err);
})
throw new Error("test-error thrown?");
console.log("this is never executed");
ghost commented 5 years ago

Refactored the errorhandling and added small adjustment. @Mondei1 Can you recheck this and see if this is ok and the PR can be merged? :)

Mondei1 commented 5 years ago

@720degreeLotus Can you look over it again? I want to merge this now -_-