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

Throw typed errors #1244

Closed
hulbert opened this issue Jul 12, 2017 · 8 comments
Closed

Throw typed errors #1244

hulbert opened this issue Jul 12, 2017 · 8 comments
Assignees
Labels
feature New functionality or improvement support Questions, discussions, and general support
Milestone

Comments

@hulbert
Copy link

hulbert commented Jul 12, 2017

As far as I can tell (primarily from reading docs), there's no obvious capability for checking instanceof on a Joi ValidationError.

This makes writing assertions with Code difficult, e.g. I'd like to able to do something like:

const Joi = require('joi')
const expect = require('code').expect

function throws() {
    Joi.assert('x', Joi.number())
}

expect(throws).to.throw(Joi.ValidationError)

Am I missing something? If not, thoughts on adding this capability?

@DavidTPate DavidTPate added the support Questions, discussions, and general support label Jul 12, 2017
@DavidTPate DavidTPate self-assigned this Jul 12, 2017
@DavidTPate
Copy link
Contributor

DavidTPate commented Jul 12, 2017

@hulbert I'm not understanding exactly what you're looking for.

I'm guessing here that you have some schemas that you want to test with some data. Given that, here's how we do it with code. The core of our testing is done in our helper, basically it does the following:

const schema = Joi.object().keys({
    username: Joi.string().alphanum().min(3).max(30).required(),
    password: Joi.string().regex(/^[a-zA-Z0-9]{3,30}$/),
    access_token: [Joi.string(), Joi.number()],
    birthyear: Joi.number().integer().min(1900).max(2013),
    email: Joi.string().email()
}).with('username', 'birthyear').without('password', 'access_token');

const result = Joi.validate({ ... }, schema);

expect(result.error.message).to.equal('Some error message');

@Marsup
Copy link
Collaborator

Marsup commented Jul 13, 2017

Sub-classing errors is a nightmare, there are already enough means to identify joi errors I'd say.

@hulbert
Copy link
Author

hulbert commented Jul 13, 2017

@DavidTPate thanks for taking a look. When testing schemas directly, we do use that pattern. However, I find many cases where we want to test validation as part of some other function call. This often applies when developing libraries. For example, if there is a constructor that validates its inputs it is useful to be able to verify when testing that the thrown error is from Joi and not something else.

class KlassThatChecksConstruction {
    constructor(settings) {
        Joi.assert(settings, schema)
    }
}

@Marsup While I don't share your opinion that error inheritance is bad, I raise this primarily because it is hard to make Code + Joi—two projects under the Hapi umbrella—interoperate nicely here.

AFAICT there is one documented method for verifying a Joi error (the .isJoi property). But it's not possible to use this with Code's throw assertion.

@Marsup Marsup added the request label Jul 14, 2017
@Marsup
Copy link
Collaborator

Marsup commented Jul 14, 2017

I can't remember what were my griefs before, but apparently ES6 class inheritance is somehow doing the right thing for Error now. I'll revisit.

@WesTyler
Copy link
Contributor

We use custom errors all over the place in our enterprise code, FWIW. This is copied exactly from our code base; I only renamed the error.

class MyError extends Error {
    constructor(...args) {
        super(...args);
    }
}

@DavidTPate
Copy link
Contributor

Thanks for the clarification @hulbert I believe I was going through a bout of temporary idiocy because looking at your issue now I understand what you were asking. 🤷‍♂️

There used to be some intricate stupidity in extending errors in ES5 and prior. Haven't looked to see if that has been taken care of with ES6 yet. The issues that I recall having were around stack traces not properly constructing and issues around the ability to iterate over certain Error properties causing for some janky code to get things working how I prefer.

@aaronjameslang
Copy link
Contributor

Looking into implementing this

@aaronjameslang
Copy link
Contributor

screen shot 2018-11-13 at 20 52 34

This was not deliberate, sorry

aaronjameslang added a commit to aaronjameslang/joi that referenced this issue Nov 15, 2018
aaronjameslang added a commit to aaronjameslang/joi that referenced this issue Nov 15, 2018
aaronjameslang added a commit to aaronjameslang/joi that referenced this issue Nov 15, 2018
aaronjameslang added a commit to aaronjameslang/joi that referenced this issue Nov 15, 2018
aaronjameslang added a commit to aaronjameslang/joi that referenced this issue Nov 17, 2018
@Marsup Marsup self-assigned this Nov 19, 2018
@Marsup Marsup added this to the 15.0.0 milestone Nov 19, 2018
@hueniverse hueniverse changed the title throw typed errors for instanceof checks? Throw typed errors Aug 10, 2019
@hueniverse hueniverse added the v16 label Aug 10, 2019
@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement support Questions, discussions, and general support
Projects
None yet
Development

No branches or pull requests

6 participants