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

fix: add missing members to parser exception type declarations #1322

Merged
merged 2 commits into from
Jan 16, 2021

Conversation

mbett7
Copy link
Contributor

@mbett7 mbett7 commented Jan 8, 2021

Fixes #1316

Also converts error constructors to classes to satisfy typing in the api_type_checking.ts test.

Note: should be able to remove the ES5 downlevel workarounds when the TS target is updated to a more modern runtime. Since babel@7 seems to be able to downlevel custom class errors to behave like ES2015 (i.e. with prototype and stacktrace intact).

@CLAassistant
Copy link

CLAassistant commented Jan 8, 2021

CLA assistant check
All committers have signed the CLA.

@bd82
Copy link
Member

bd82 commented Jan 12, 2021

Thanks @mbett7 👍 I will have a look at this PR on the weekend


NoViableAltException.prototype = Error.prototype
/* istanbul ignore next - V8 workaround to remove constructor from stacktrace when typescript target is ES5 */
if (Error.captureStackTrace) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen on Safari / Firefox ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think I now understand this workaround removes some redundant line from the stack trace.
Not fixes the whole stack trace, so its only a minor issue on none V8 runtimes.

@bd82
Copy link
Member

bd82 commented Jan 16, 2021

Thanks @mbett7 👍

This looks great, I have only one concern:
Would the exception stack traces work on none V8 JS engines?

@bd82
Copy link
Member

bd82 commented Jan 16, 2021

Not sure how important this is however...
As I hope to upgrade to a newer ECMAScript version sometime in the next few months (time permitting...)

@bd82 bd82 merged commit 9045686 into Chevrotain:master Jan 16, 2021
@mbett7
Copy link
Contributor Author

mbett7 commented Jan 16, 2021

Thanks @mbett7 👍

This looks great, I have only one concern:
Would the exception stack traces work on none V8 JS engines?

I only tested in IE11, Chrome & Nodejs... Should have tested it in Firefox as well. Since it looks like the constructors remain in the stacktrace (also the babel downlevel doesn't turn out so well). Not sure about Safari don't have an iOS environment.

Test Code

class CustomException extends Error {
    constructor(message) {
        super(message);
        this.name = 'CustomException';
        Object.setPrototypeOf(this, new.target.prototype);
        if (Error.captureStackTrace) {
            Error.captureStackTrace(this, this.constructor);
        }
    }
}

var customError = new CustomException('msg');
console.log('CustomException.name:', customError.name);
console.log('instanceof CustomException:', customError instanceof CustomException);
console.log('instanceof Error:', customError instanceof Error);
console.log('CustomException log: ', customError);

function throwCustomError() {
    throw new CustomException('msg');
}
try {
    throwCustomError();
} catch (e) {
    console.log('Catch thrown CustomException:', e);
}

Test Output
Chrome (no downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
    at <anonymous>:12:19
Catch thrown CustomException: CustomException: msg
    at throwCustomError (<anonymous>:19:11)
    at <anonymous>:22:5

Chrome (Typescript ES5 downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
    at <anonymous>:29:19
Catch thrown CustomException: CustomException: msg
    at throwCustomError (<anonymous>:35:11)
    at <anonymous>:38:5

Chrome (Babel ES5 downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
    at <anonymous>:51:19
Catch thrown CustomException: CustomException: msg
    at throwCustomError (<anonymous>:58:9)
    at <anonymous>:62:3

IE11 (Typescript ES5 downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
   "CustomException log: "
   {
      [functions]: ,
      __proto__: { },
      description: "msg",
      message: "msg",
      name: "CustomException"
   }
Catch thrown CustomException: CustomException: msg
   "Catch thrown CustomException"
   {
      [functions]: ,
      __proto__: { },
      description: "msg",
      message: "msg",
      name: "CustomException",
      stack: "CustomException: msg
   at throwCustomError (eval code:35:5)
   at eval code (eval code:38:5)
   at Global code (Unknown script code:5:1)"
   }

IE11 (Babel ES5 downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
   "CustomException log: "
   {
      [functions]: ,
      __proto__: { },
      description: "msg",
      message: "msg",
      name: "CustomException"
   }
Catch thrown CustomException: CustomException: msg
   "Catch thrown CustomException"
   {
      [functions]: ,
      __proto__: { },
      description: "msg",
      message: "msg",
      name: "CustomException",
      stack: "CustomException: msg
   at throwCustomError (eval code:58:3)
   at eval code (eval code:62:3)
   at Global code (Unknown script code:5:1)"
   }

Firefox (no downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
    CustomException debugger eval code:3
    <anonymous> debugger eval code:12
Catch thrown CustomException: CustomException: msg
    CustomException debugger eval code:3
    throwCustomError debugger eval code:19
    <anonymous> debugger eval code:22

Firefox (Typescript ES5 downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
    CustomException debugger eval code:19
    <anonymous> debugger eval code:29
Catch thrown CustomException: CustomException: msg
    CustomException debugger eval code:19
    throwCustomError debugger eval code:35
    <anonymous> debugger eval code:38

Firefox (Babel ES5 downlevel)

CustomException.name: CustomException
instanceof CustomException: true
instanceof Error: true
CustomException log:  CustomException: msg
    _construct debugger eval code:17
    Wrapper debugger eval code:15
    _createSuperInternal debugger eval code:9
    CustomException debugger eval code:37
    <anonymous> debugger eval code:51
Catch thrown CustomException: CustomException: msg
    Wrapper debugger eval code:15
    _createSuperInternal debugger eval code:9
    CustomException debugger eval code:37
    throwCustomError debugger eval code:58
    <anonymous> debugger eval code:62

@mbett7 mbett7 deleted the fix-1316 branch March 13, 2021 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Public members missing from MismatchedTokenException type declaration
3 participants