failAction: detailedError to log, defaultError to response

Support plan

which support plan is this issue covered by? (e.g. Community, Core, Plus, or Enterprise): Community, Core
is this issue currently blocking your project? (yes/no): yes
is this issue affecting a production system? (yes/no): no

Context

node version: v12 v14
module version: 20.0.3
environment (e.g. node, browser, native): node
used with (e.g. hapi application, another framework, standalone, ...): hapi app
any other relevant information: original issue failAction: detailedError to log, defaultError to response #4040

What problem are you trying to solve?

It is impossible to log detailedError to the console, defaultError to the response.

Current implementation allows:

failAction: 'error'
// defaultError to log and to response

failAction: (request, h, err) => { request.log('error', err); throw err }
// detailedError to log and to response

There is no way to access both detailedError and defaultError in the handler.

For example:

defaultError: Error: Invalid request query input
detailedError: Error [ValidationError]: "foobar" is not allowed

Do you have a new or modified API suggestion to solve the problem?

https://github.com/hapijs/hapi/blob/master/lib/toolkit.js#L117-L135

if (failAction === 'log-and-error') {
    request._log(options.tags, options.details || err);
    throw err;
}

import Joi from 'joi'; // 17.4.0

server.route({
  method: 'GET',
  path: '/test',
  options: {
    validate: {
      query: Joi.object({
        foo: Joi.string().required(),
      }),
      failAction: 'error',
      // response: {"statusCode":400,"error":"Bad Request","message":"Invalid request query input"}
      failAction: 'log',
      // log to console: Error: Invalid request query input
      failAction(request, h, err) {
        request.log('error', err);
        throw err;
      },
      // log to console: Error [ValidationError]: "foo" is not allowed to be empty
      // response: {"statusCode":400,"error":"Bad Request","message":"\"foo\" is not allowed to be empty","validation":{"source":"query","keys":["foo"]}}
    },
  },
  handler() {
    return 'ok';
  },
});

There is no way to response by short error {"statusCode":400,"error":"Bad Request","message":"Invalid request query input"} and log to console detailed error Error [ValidationError]: "foo" is not allowed to be empty.

Yeah, I always use the last one and I do it at the server level:

failAction(request, h, err) {
    request.log('error', err);
    throw err;
},

The other modes seem pretty pointless to me. I think you'd always want the detailed error in the logs and the only thing that really needs configuration is whether to return the detailed error in the response.

I believe this is a valid feature request. Even though you could throw a Boom.badRequest error as Eran mentioned you lose the message generated by Joi for the defaultError. This not a good solution if you have a global failAction setting in your server configuration IMO because there is no way to make an error message that is still a bit precise without providing the full information detailedError has.

We could handle this in a non-breaking way by adding args: [options.details || err, err] here. I'm not a big fan of it because the first arg could be either the detailedError or the defaultError. In the case where we don't have a detailedError both arg would be the same. At least this has the benefit of being a non-breaking change.

If we go that route and don't want to publish a breaking change, I'd suggest we also keep an issue open for the next major version release to replace it with: args: [options.details, err].

In the end the failAction signature would be:

failAction(request, h, detailedError | defaultError, defaultError) {}

That's a good thought. Another approach could be to tack the default error onto the detailed error in this case so that it could be accessed:

failAction(request, h, err) {
    request.log('error', err);
    throw err.defaultError;
}

Another somewhat related problem is how the default error message includes "request query input", which is useful to know, but the detailed error message doesn't include that source info.

So in my logs, for example, I have to choose between:

With failAction : 'log':

Debug: validation, error, query 
    Error: Invalid request query input
    at Object.internals.input (/[redacted]/node_modules/@hapi/hapi/lib/validation.js:148:74)
    at async Request._lifecycle (/[redacted]/node_modules/@hapi/hapi/lib/request.js:370:32)
    at async Request._execute (/[redacted]/node_modules/@hapi/hapi/lib/request.js:279:9)

With failAction : (request, h, error) => { request.log('error', error); throw error; }:
```
Debug: error 
    ValidationError: "ignoreNotFound" must be []
```

Except, option 1 isn't really an option for me, because failAction : 'log' disables enforcement of the validation, which is basically never what I want. Although, I see how it could be useful in some rare cases.

It's also kind of odd how option 1 includes a (relatively useless) stack trace while option 2 does not.

Personally, all of this stuff is my biggest pain point with hapi right now.

@devinivy

Good idea @devinivy . I didn't think of this one. Would you consider it a transitional step before a breaking change or the final implementation?

Thanks @sholladay . Interesting stuff to consider as well.

I'm really happy that this feature has found followers. I'm looking forward to have it implemented in any from proposed option.

This is exactly what I was looking for.

I implemented this behavior but it was a bit hacky without having the defaultError in the failAction method.

This implementation is TS and typesafe. That is the reason for some (maybe unnecessary) checks:

import {ValidationError} from "joi";

failAction: (request, _h, err): null => {
    if (Boom.isBoom(err)) {
      const validation = err.output.payload.validation as
        | { source?: string }
        | undefined;
      const source = validation?.source ?? '';
      if (err instanceof ValidationError) {
        request.log(['validation', 'error', source], err.message);
      }
      throw Boom.badRequest(`Invalid request ${source} input`);
    } else {
      // should never be the case, because a failAction is always called with a Boom error
      throw Boom.badRequest('Invalid request input');
    }
}

So in short maybe:

failAction: (request, h, err) => {
    const source = err.output.payload.validation?.source
    request.log(['validation', 'error', source], err.message);
    throw Boom.badRequest(`Invalid request ${source} input`);
}

Resolved with v21 #4386

Navigation Menu

Repository navigation

failAction: detailedError to log, defaultError to response #4229

Support plan

Context

What problem are you trying to solve?

Do you have a new or modified API suggestion to solve the problem?

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

failAction: detailedError to log, defaultError to response #4229

Description

Support plan

Context

What problem are you trying to solve?

Do you have a new or modified API suggestion to solve the problem?

Activity

mahnunchik commented on Feb 10, 2021

sholladay commented on Feb 13, 2021

Nargonath commented on Feb 19, 2021

devinivy commented on Feb 19, 2021

sholladay commented on Feb 19, 2021

Nargonath commented on Feb 24, 2021

mahnunchik commented on Feb 24, 2021

ghdoergeloh commented on Jan 12, 2022

devinivy commented on Nov 7, 2022

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions