output.payload.attributes is not exposed in the .d.ts file

watson commented 3 years ago

Support plan

is this issue currently blocking your project? (yes/no): no
is this issue affecting a production system? (yes/no): no

Context

node version: 14.15.1
module version with issue: 9.1.0
last module version without issue: 7.4.11
environment (e.g. node, browser, native): node
used with (e.g. hapi application, another framework, standalone, ...): hapi application and standalone
any other relevant information:

What are you trying to achieve or the steps to reproduce?

const Boom = require('@hapi/boom')
const err = Boom.badRequest()
console.log(err.output.payload.attributes)

What was the result you got?

TypeScript will complain about the last line with the following error:

Property 'attributes' does not exist on type 'Payload'.ts(2339)

What result did you expect?

No complaint from TypeScript.

In 7.4.11 we used @types/hapi__boom which did expose the attributes property on Payload. However, since v8.0.0 types have been built in to this package directly, but the Payload object doesn't have any mention of an attributes property.

Nargonath commented 3 years ago

attributes seems to only concern the unauthorized boom error: https://github.com/hapijs/boom/blob/b25ee43255b939a28e99bae697bb82f73c0e8129/lib/index.js#L141 in latest version but also in 7.4.11 as far as I can tell . The typings seems to also reflect that in the latest version but not in the deprecated @types/hapi__boom package. If you look in the comments on @types/hapi__boom it's mentioned that way as well.

From what I can see you benefited from an error in the typings which has now been fixed since v8. It is unlikely that this will go back to how it was. One thing I'm curious though is: how did get data into badRequest().output.payload.attributes?

I'll close this for now but we can continue to discuss this and we can reopen it if I made a mistake.

watson commented 3 years ago

how did get data into badRequest().output.payload.attributes?

We simply manually populated the property:

https://github.com/elastic/kibana/blob/1c16bcf8512851748ec69b583172ec73029f6238/src/core/server/http/router/response_adapter.ts#L132-L141

However, this might not have been a good idea. I'm currently investigating why as I didn't write that code

watson commented 3 years ago

After a bit of digging, I found that the hapi documentation actually allow the use of error.output.payload.any_custom_property: https://hapi.dev/api/?v=20.0.3#error-transformation

Populating error.output.payload with custom properties seem to be the only reasonable way to have the JSON http response populated with custom properties besides statusCode, error, and message.

So shouldn't this also be allowed by TypeScript?

kanongil commented 3 years ago

The boom types should be updated to handle this. I expect a [key: string]: unknown property on the Payload interface should do the trick.

Nargonath commented 3 years ago

I agree with @kanongil. Do you want to make a PR for it @watson?

watson commented 3 years ago

PR: #279

hapijs / boom