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 custom error (ES6) #596

Closed
theofidry opened this issue Jan 24, 2016 · 24 comments
Closed

Throw custom error (ES6) #596

theofidry opened this issue Jan 24, 2016 · 24 comments

Comments

@theofidry
Copy link

Given a custom error:

export default class DummyError extends Error {
    /**
     * @param {string} [message='']
     */
    constructor(message = '') {
        super(message);

        this.name = 'DummyError';
        this.message = message;

        if (Error.hasOwnProperty('captureStackTrace')) {
            Error.captureStackTrace(this, this.constructor);
        }

        this.stack = (new Error(message)).stack;
    }
}

And the following function:

function throwDummyError() {
    throw new DummyError();
}

The following doesn't work:

assert.throw(throwDummyError, DummyError);

And instead is throwing:

AssertionError: expected [Function: DummyError] to throw 'DummyError' but 'my error message' was thrown
@keithamus
Copy link
Member

Hey @theofidry thanks for the issue.

That should definitely work. The way we handle errors is getting better in 4.x.x, so I'd first ask you to reproduce the error using the 4.x.x branch. Could you try to reproduce using the tip of 4.x.x please? Steps to do so:

  1. npm install chaijs/chai#4.x.x
  2. Run your tests
  3. Report results 😄

@theofidry
Copy link
Author

Indeed this was using the 3.4.1 version, should have said so from the beginning. I'm going to test it with the 4.x.x branch and come back to you.

@keithamus
Copy link
Member

Hey @theofidry have you had a chance to try out 4.x.x yet? It'd be great to hear back from you 😄 👂

@theofidry
Copy link
Author

Unfortunately no, busy times 😞. But think I will be able to do it today or during the weekend...

@keithamus
Copy link
Member

Great! I look forward to seeing how you go with it 😄

@theofidry
Copy link
Author

Nope doesn't work :( haircvt/serializerjs#14

@theofidry
Copy link
Author

Ha, looks like the npm install does not pass in Travis, locally I have:

 1) SerializerInterface As an interface, it should not implement anything:

      AssertionError: expected [Function: deserialize] to throw 'UnimplementedMethodError' but 'UnimplementedMethodError: Unimplemented "deserialize()" method' was thrown
      + expected - actual

      -UnimplementedMethodError: Unimplemented "deserialize()" method
      +UnimplementedMethodError

      at Function.assert.throws (node_modules/chai/lib/chai/interface/assert.js:1028:52)
      at Context.<anonymous> (SerializerInterfaceTest.js:29:9)

@keithamus
Copy link
Member

Okay. Thanks very much for trying it out.

It's interesting that it says but 'UnimplementedMethodError: Unimplemented "deserialize()" method' was thrown. What is the value of UnimplementedMethodError.prototype.name? I think it checks based on that.

If you have the time to investigate what is happening, or perhaps making a small reduced test case. That'd be amazing!

@theofidry
Copy link
Author

It's UnimplementedMethodError, no doubt on it as it's hard coded: UnimplementedMethodError.#L22.

Will try to do a reduced test case with the snippet above.

@keithamus
Copy link
Member

👍 awesome thanks 😄

@lucasfcosta
Copy link
Member

I'll take another look on the assert interface today and do some polishment on it.

@theofidry could give me more details on this?
I didn't exactly understand how that function resulted on this output:

AssertionError: expected [Function: DummyError] to throw 'DummyError' but 'my error message' was thrown

If I'm not missing anything here, that output doesn't correspond to the example code you posted (where does that 'my error message' comes from?). It would be great if could post the whole code which generated that or a more complete example of this.

Hopefully we've got every possible combination of arguments correctly handled on #683, so if there's any error on this it may be due to the lack of polishment on the assert interface.

Thanks for your time @theofidry, I'll keep this one for now so we can investigate it further.

@theofidry
Copy link
Author

Hi @lucasfcosta, I'll try to take a look at it again during the weekend :)

@lucasfcosta
Copy link
Member

Hello guys, I just want to update everyone on this:

I already finished the polishments on the Assert interface, however I found another edge case which needs to be covered thanks to one of the Assert interface tests regarding empty strings as error messages.

I'm working to fix it and I plan on sending a PR for that tomorrow. I may need to change the check-error module too.

@meeber
Copy link
Contributor

meeber commented Jul 1, 2016

@lucasfcosta Was this resolved via #742?

@lucasfcosta
Copy link
Member

lucasfcosta commented Jul 1, 2016

@meeber I think it should have been, but I cannot guarantee, I'd like to see @theofidry's example and/or wait for him to test it with his whole code before closing this one. I don't have enough information for now, since I can't see the whole code involved.
Let's keep this one open until next week and see if we can get any more info.

@meeber
Copy link
Contributor

meeber commented Aug 17, 2016

Closing this. Can always reopen if there's still an issue.

@meeber meeber closed this as completed Aug 17, 2016
@samiraguiar
Copy link

samiraguiar commented Feb 18, 2017

I think I might have a similar issue.

I have the following custom error (Typescript code below):

const message: string = `error message`;

export class InvalidReferenceFormatError extends Error {
    constructor() {
        super(message);
        this.name = "InvalidReferenceFormatError";
        this.stack = (<any> new Error()).stack;
    }
}

And the assertion:

expect(() => { throw new InvalidReferenceFormatError() }).to.throw(InvalidReferenceFormatError);

This results in:
AssertionError: expected [Function] to throw 'InvalidReferenceFormatError' but 'InvalidReferenceFormatError: error message' was thrown

Is a message mandatory for custom errors?
Using chai version 3.5.0.

@meeber
Copy link
Contributor

meeber commented Feb 19, 2017

@samiraguiar The following works for me using Chai v3.5 and Node v7.5 (without TypeScript/transpilation):

const message = `error message`;                                                
                                                                                
class InvalidReferenceFormatError extends Error {                               
    constructor() {                                                             
        super(message);                                                         
        this.name = "InvalidReferenceFormatError";                              
        this.stack = (new Error()).stack;                                       
    }                                                                           
}                                                                               
                                                                                
it('blah', function () {                                                        
  expect(() => { throw new InvalidReferenceFormatError() }).to.throw(InvalidReferenceFormatError);                                                              
});

My best guess is that your code is being transpiled to something that's incompatible with the throw logic.

@samiraguiar
Copy link

samiraguiar commented Feb 19, 2017

You're right, @meeber, the transpiled code is to blame.

Because I'm using transpilation to es5, it does some workarounds for inheritance. Here's the offending transpiled code:

require("mocha");
var chai = require("chai");

// ----- generated code -----
var __extends = (this && this.__extends) || function (d, b) {
    for (var p in b) if (b.hasOwnProperty(p)) d[p] = b[p];
    function __() { this.constructor = d; }
    d.prototype = b === null ? Object.create(b) : (__.prototype = b.prototype, new __());
};

var message = "error message";

var InvalidReferenceFormatError = (function (_super) {
    __extends(InvalidReferenceFormatError, _super);
    function InvalidReferenceFormatError() {
        var _this = _super.call(this, message) || this;
        _this.name = "InvalidReferenceFormatError";
        _this.stack = new Error().stack;
        return _this;
    }
    return InvalidReferenceFormatError;
}(Error));
// ---------------------

it('bla', function () {
    chai.expect(function () { throw new InvalidReferenceFormatError(); }).to.throw(InvalidReferenceFormatError);
});

Out of curiosity, any idea why it doesn't work in this case?

@meeber
Copy link
Contributor

meeber commented Feb 19, 2017

@samiraguiar Yeah, the transpiled version is creating the error in a weird way in which the thrown error object isn't considered an instanceof InvalidReferenceFormatError. Consider the following example that doesn't involve Chai:

var meh = new InvalidReferenceFormatError();
console.log(meh instanceof InvalidReferenceFormatError);

The above prints false for the transpiled version, but true for the native version. The .toString() of the thrown error identifies itself as "InvalidReferenceFormatError: error message", thus the failed assertion message when using Chai, but that can't be relied upon when checking instance type.

@samiraguiar
Copy link

samiraguiar commented Feb 20, 2017

Thanks @meeber, it seems that this is a known issue. I'll just have to do some workarounds to get this working.

@keithamus
Copy link
Member

This is a known issue among all transpilers as ES5 environments don't support extending for builtins like Array, Error etc. The same is true for Babel (docs here). This is pretty much a dupe of #909, #773 and I think a few others.

It might be worth documenting this in the .throws example by now. In #909 I said:

I'll close this; while we could document it I somewhat don't think it is not our responsibility to document issues with other tools and would be burdensome to do so.

Which I still a valid and justified sentiment, but - pragmatically - we're spending more time on circling round duplicate issues that documenting this issue within our own docs might just be the less time-consuming option for this instance.

@HoldYourWaffle
Copy link

I just stumbled upon this issue again. Unfortunately it's not currently mentioned in the documentation, even though #938 was apparently merged into #920.

@neo-gravity-pawn
Copy link

In the breaking changes documentation for typescript 2.1 (https://github.com/Microsoft/TypeScript-wiki/blob/master/Breaking-Changes.md#extending-built-ins-like-error-array-and-map-may-no-longer-work) it is suggested, to explicitly set the prototype of the custom error to allow for typechecks via instanceof:

class FooError extends Error {
    constructor(m: string) {
        super(m);
        // Set the prototype explicitly.
        Object.setPrototypeOf(this, FooError.prototype);
    }
    sayHello() {
        return "hello " + this.message;
    }
}

After following this, it seems expect(()=>{throw new FooError()}).to.throw(FooError) works as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants