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

Exception support for upgrading #19595

Open
1 of 3 tasks
basvangijzel-DA opened this issue Jul 15, 2024 · 2 comments · Fixed by #20003
Open
1 of 3 tasks

Exception support for upgrading #19595

basvangijzel-DA opened this issue Jul 15, 2024 · 2 comments · Fixed by #20003
Assignees

Comments

@basvangijzel-DA
Copy link
Contributor

basvangijzel-DA commented Jul 15, 2024

  • Discuss with Curtis if needed
  • Add typechecking support
  • Upload check that this doesn't end the world
@samuel-williams-da
Copy link
Contributor

We document that exceptions should be defined in their own package and add a compiler warning.
If you want to “upgrade” an exception, you create a new exception type. The only thing to be aware of is that the old code won’t be able to catch it.

@dylant-da
Copy link
Contributor

dylant-da commented Sep 17, 2024

Discussed in call with Paul and Samuel:

First, reproduce issue

  • interface exercise -> produces exceptions -> upgraded exception is different, is not caught by existing case statement, falls through to wildcard

For 2.x

  • exceptions should be put into the same package as interfaces
  • Update docs to explain
  • exceptions needing to be in their own package
  • What goes wrong if you don’t do it

For 3.x

  • Make catch primitive upgrade/downgrade exceptions
  • In the event of a failed downgrade, abort the transaction
  • Implement upgrade/downgrade logic within toAnyException
  • Feasible, but may end up thorny, would prefer 3.x

@dylant-da dylant-da linked a pull request Sep 25, 2024 that will close this issue
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 a pull request may close this issue.

3 participants