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

v12-alpha: The raise call started recreating exn, which breaks try/catch #6929

Open
DZakh opened this issue Aug 3, 2024 · 6 comments · May be fixed by #6933
Open

v12-alpha: The raise call started recreating exn, which breaks try/catch #6929

DZakh opened this issue Aug 3, 2024 · 6 comments · May be fixed by #6933
Labels
Milestone

Comments

@DZakh
Copy link
Contributor

DZakh commented Aug 3, 2024

Since it accepts the exn type, I expect it to simply generate throw exn instead of recreating an exception instance, which might break on exn having a Js error.

v11: https://rescript-lang.org/try?version=v11.1.3&code=C4JwngBA3gUBECkDOA6AogDwHYpAQwEskBTNEEAexAAoAiAMwotoEoYBfCAYz2C4AtoMAD4Ri2CAF4AfBHxFi1cVjbsYMcV2IAHYAQpYIAFWJJg60JFjx5JaibOruvAUNHKps24uWqgA

v12: https://rescript-lang.org/try?version=v12.0.0-alpha.1&code=C4JwngBA3gUBECkDOA6AogDwHYpAQwEskBTNEEAexAAoAiAMwotoEoYBfCAYz2C4AtoMAD4Ri2CAF4AfBHxFi1cVjbsYMcV2IAHYAQpYIAFWJJg60JFjx5JaibOruvAUNHKps24uWqgA

@aspeddro
Copy link
Contributor

aspeddro commented Aug 3, 2024

I think we can change Js.Exn.raiseError function to:

let raiseError = str => raise(Failure(str))

Playground

@aspeddro
Copy link
Contributor

aspeddro commented Aug 3, 2024

There is no idiomatic way to throw other errors like EvalError. In fact, there never was.

@aspeddro
Copy link
Contributor

aspeddro commented Aug 4, 2024

Something we can support is exception name as error name.

exception EvalError(string)


raise(EvalError("foo"))

compile to:

throw new EvalError("foo", {
        cause: {
          RE_EXN_ID: "EvalError",
          _1: "foo"
        }
      });

@DZakh
Copy link
Contributor Author

DZakh commented Aug 4, 2024

That's not the point. How we throw an error doesn't matter since it might come from an unexpected runtime exception. It still shouldn't break in the case. If the code is in Js_dump, I can probably fix it myself.

@DZakh DZakh linked a pull request Aug 4, 2024 that will close this issue
@DZakh
Copy link
Contributor Author

DZakh commented Aug 4, 2024

This is how I see the solution for the issue #6933

@DZakh
Copy link
Contributor Author

DZakh commented Aug 4, 2024

I can implement it myself during the week

@cknitt cknitt added this to the v12 milestone Aug 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants