"Unhandled Error" for errors that have been handled properly.

[luncht1me]

See #40
This needs to be reopened and checked over.

My comment at the bottom shows that this bug is still present.

When we're writing remote methods on our models and explicitly returning an error with a statusCode and a message back up to the client, strong-error-handler should not be saying that there is an Unhandled Error, as we are clearly handling it with our methods, by the method defined in the loopback docs

Maybe if I get some free time later this week I can look into it, but for now one of the dedicated devs should really check this out.

Using the latest, loopback 3.6.0 locally and strongloop 6.0.3 globally.

---

Proposal from @bajtos on how to fix this issue - see #54 (comment)

As I see it, there are two parts

1. which errors to report (4xx, 5xx, or maybe only those with no explicit statusCode property)
2. what message to print

As for 1), we have deliberately decided to always print all errors, nothing's changing there.

As for 2), I agree that prefixing the log message with "Unhandled error" is misleading. I am proposing to change the message to Request {verb} {path} failed: {error} - see https://github.com/strongloop/strong-error-handler/blob/323cd4dfbffa3425d6d6bd00bd9ba93fbede5342/lib/logger.js

This issue is waiting for a volunteer to open a pull request and make the proposed change.

[jmls]

yup, hit this as well

[bajtos]

Thank you for providing more information, I am understanding the problem better now.

As I see it, there are two parts

1. which errors to report (4xx, 5xx, or maybe only those with no explicit statusCode property)
2. what message to print

As for 1), we have deliberately decided to always print all errors, nothing's changing there.

As for 2), I agree that prefixing the log message with "Unhandled error" is misleading. I am proposing to change the message to Request {verb} {path} failed: {error} - see https://github.com/strongloop/strong-error-handler/blob/323cd4dfbffa3425d6d6bd00bd9ba93fbede5342/lib/logger.js

Thoughts?

[jmls]

I don't know if this is related, but using promises in remotes also generates this type of error

Workspace.save = (workspace,options) => {
        return new Promise((resolve,reject) => {
           Promise.all([])
            .then((result) => {
                return resolve(result);
            })
            .catch((e) => {
                return reject(e);
            })
        });
    };

spews out Unhandled error for request PUT /Workspaces: StatusCodeError: 422 .... on the console.

Nope, sorry, I very definitely handled the error ;)

[bajtos]

using promises in remotes also generates this type of error

Rejecting a promise with an error is the same as calling cb(err) or throw err, so yes, it's related.

This may be subjective, but your code did not handle the error, it merely passed it down the .then chain for somebody else to handle it: LoopBack's strong-remoting block for handling errors reported by model methods and then strong-error-handler to render the error to the HTTP response.

I don't want to argue about wording and definitions. See my earlier #54 (comment) for an actionable proposal.

[jmls]

ok, so I'm coming back to this because I am confused ;) Doesn't take a lot...

how am I supposed to return the error to the browser from my remote method using promises ?

 myModel.someRemote = (id: number, options) => {
        return Promise.reject(new Error('foo'));
    };

  myModel.remoteMethod(
        'someRemote',
        {
            description: 'someRemote',
            accepts: [
                { arg: 'id', type: 'number', required: true },
                { arg: 'options', type: 'object', http: 'optionsFromRequest' }
            ],
            returns: { arg: 'data', type: 'array', root: true },
            http: { path: '/:id/someRemote', verb: 'put' }
        });

[bajtos]

@jmls

how am I supposed to return the error to the browser from my remote method using promises?

Your code looks correct to me. The remote method returns a promise that's eventually rejected, strong-remoting will treat this rejection as an error and strong-error-handler will convert this error into an HTTP error response (and log a message).

[a-legrand]

Same here,
using loopback-component-storage trying to upload a document that exceed max file size

[kennethpdev]

just bumped on this problem. Any other workarounds? I've been looking even at page 2 of google and no joy. Still getting the annoying Unhandled error for request GET error even though its just a simple error test.

const HttpErrors = require('http-errors');
User.verify= (callback) => {
  callback(new HttpErrors.BadRequest('Unknown user.'));
};

Dependencies

"loopback": "^3.26.0",
"loopback-boot": "^3.2.0",
"loopback-component-explorer": "^6.4.0",
"loopback-connector-postgresql": "^3.5.0",

[bajtos]

See my earlier #54 (comment) for an actionable proposal.

I agree that prefixing the log message with "Unhandled error" is misleading. I am proposing to change the message to Request {verb} {path} failed: {error} - see https://github.com/strongloop/strong-error-handler/blob/323cd4dfbffa3425d6d6bd00bd9ba93fbede5342/lib/logger.js

Any volunteers to contribute that change?

[bajtos]

Published the changes in strong-error-handler@4.0.0.

[andresmatasuarez]

I'm aware of the changes introduced in strong-error-handler@4.0.0 and I do like the logging of "Request failed" instead of the confusing "Unhandled error", but if you still don't want anything logged at all, you can do something like the following:

middlewares.json:

  "final:after": {
    "./middlewares/skipHandledErrors": {}, <<<<<<<<<<<<<< put this line here
    "strong-error-handler": {}
  }

Create the skipHandledErrors.js file:

module.exports = function() {
  return function skipHandledErrors(err, req, res, next) {
    return err.hasBeenHandled ? null : next(err);
  };
};

Now, for each error you handle explicitly, just add a property hasBeenHandled=true to it, e.g:

MyModel.afterRemoteError('prototype.doSomething', async function(ctx) {
    if (ctx.error.statusCode === 401) {
      ctx.res.redirect('http://www.myapp.com/login');

      ctx.error.hasBeenHandled = true;

      return;
    }

    throw ctx.error;
  });

it's not ideal but it's not the end of the world either.