Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@hapi/validate schemas in the context of hapi-swagger use #4246

Open
Marsup opened this issue Apr 13, 2021 · 2 comments
Open

@hapi/validate schemas in the context of hapi-swagger use #4246

Marsup opened this issue Apr 13, 2021 · 2 comments
Labels
support Questions, discussions, and general support

Comments

@Marsup
Copy link
Contributor

Marsup commented Apr 13, 2021

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.16.0
  • module version: 20.1.0
  • environment (e.g. node, browser, native): node
  • used with (e.g. hapi application, another framework, standalone, ...): hapi-swagger@14.1.0
  • any other relevant information:

How can we help?

I'm starting an issue here, but I don't really know if it's up to hapi or hapi-swagger to fix the issue, I'd say hapi, but you may disagree.
There is currently an error thrown from hapi-swagger that looks like this:

Error: Cannot mix different versions of joi schemas
    at new module.exports (./node_modules/@hapi/hoek/lib/error.js:23:19)
    at module.exports (./node_modules/@hapi/hoek/lib/assert.js:20:11)
    at Object.exports.isSchema (./node_modules/joi/lib/common.js:132:5)
    at Object.utilities.isJoi (./node_modules/hapi-swagger/lib/utilities.js:236:24)
    at Object.utilities.hasJoiChildren (./node_modules/hapi-swagger/lib/utilities.js:246:19)
    at ./node_modules/hapi-swagger/lib/paths.js:291:21
[...]

The root cause is coming from this line. When hapi-swagger is checking that all the schemas are joi objects, this one is not, so the throw is totally expected.

Now I see 2 solutions to that :

  • use the validator to create that schema so that it uses the correct joi version, but it doesn't seem easy to add the allow(null) part if we're not sure that we're getting a joi object back from the validator
  • keep the false as it is and do some special treatment during the validation, but it has imho one major drawback, it's going to be hard to throw joi-compatible errors, which could cause some problems for people catching errors in failAction or any other hook

What do you think?

@Marsup Marsup added the support Questions, discussions, and general support label Apr 13, 2021
@kanongil
Copy link
Contributor

kanongil commented Apr 13, 2021

Hmm… yeah, this does seem strange for hapi to expose Validate for run-time validations. Eg. there is no way to localize the generated error.

Ideally, hapi would use the passed validator. Maybe the correct solution is to allow validators to provide a "nothing-allowed" schema method? Maybe registered something like this?

server.validator(Joi, {
    empty: Joi({}).allow(null)
});

@devinivy
Copy link
Member

devinivy commented Apr 14, 2021

Eg. there is no way to localize the generated error.

keep the false as it is and do some special treatment during the validation, but it has imho one major drawback, it's going to be hard to throw joi-compatible errors

I wonder if this drawback can be addressed by users using their validator rather than the convenient false shorthand, if it's important to them. If that is the case, then I think the second option ("keep the false as it is and do some special treatment") would be a nice way to handle it. We may be able to go a step further and give some basic guarantees about the error format for this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
support Questions, discussions, and general support
Projects
None yet
Development

No branches or pull requests

3 participants