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

Allow JavaScriptException to be subclassed #1093

Merged
merged 10 commits into from Mar 6, 2022
Merged

Allow JavaScriptException to be subclassed #1093

merged 10 commits into from Mar 6, 2022

Conversation

ghost
Copy link

@ghost ghost commented Feb 16, 2022

No description provided.

Tim Cassidy added 2 commits February 16, 2022 13:48
…ding additional metadata (like filename) to exception.
…t making Location protected instead of private)
@ghost
Copy link
Author

ghost commented Feb 16, 2022

On the subject of exception/error handling - it would be nice to have the managed exception's callstack/stack-trace when javascript exceptions are thrown.

here is one minor update that would debugging a lot:

        public static void ThrowReferenceError(Realm realm, string name)
        {
            var message = name != null ? name + " is not defined" : null;
            throw new JavaScriptException(realm.Intrinsics.ReferenceError, message, new System.Exception("managed exception"));
        }

Now this exception will have an inner exception containing the callstack from within the JINT managed code.

You could even wrap the above change with a

#if DEBUG

if you only wanted that inner managed exception callstack to be available when using a debug assembly.


public JavaScriptException(ErrorConstructor errorConstructor) : base("")
public JavaScriptException(ErrorConstructor errorConstructor) : base("")
Copy link
Collaborator

Choose a reason for hiding this comment

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

weird indent

Copy link
Author

Choose a reason for hiding this comment

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

ah - yeah - sorry - I wasn't sure if your project preferred tabs or spaces for indenting - I actually didn't realize visual studio had used spaces - so I just updated it with tabs.

Visual Studio does this weird thing where by default for C# it does whatever your preference is (i.e. for me it is tabs - not spaces) - but if it sees spaces for indenting in the file - it will use that instead of whatever you've set for your preference.

Copy link
Author

Choose a reason for hiding this comment

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

I just realized that my usage of tabs (instead of spaces) made the diff look worse - so I just reverted it back to spaces for indent since, at least in this file, that appears to be the convention.

@lahma
Copy link
Collaborator

lahma commented Feb 17, 2022

At the same time I don't see much harm in this and then again I'm not sure if custom exceptions should derive from Jint's exceptions as they "are outside the engine". Many BLC exceptions allows this so there's that. Pinging @sebastienros for confirmation.

@ghost
Copy link
Author

ghost commented Feb 24, 2022

@sebastienros - this should fix the bug with extension methods that overload mundane methods

Sorry - I failed to check the name requested matches.

@lahma lahma changed the title Allow JavaScriptException to be subclassed. Allow JavaScriptException to be subclassed Mar 6, 2022
@lahma lahma merged commit ea62f4e into sebastienros:main Mar 6, 2022
@lahma
Copy link
Collaborator

lahma commented Mar 6, 2022

Thank you for your contribution! I did small stylistic tweaks before merging, I hope you don't mind.

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.

None yet

1 participant