Skip to content

fix(js/ts/python/dart/beam): raise typed exceptions from invalidArg and invalidOp - #5152

Open
leonitousconforti wants to merge 1 commit into
fable-compiler:mainfrom
leonitousconforti:fix-invalidarg-invalidop-types
Open

leonitousconforti wants to merge 1 commit into
fable-compiler:mainfrom
leonitousconforti:fix-invalidarg-invalidop-types

Conversation

@leonitousconforti

Copy link
Copy Markdown

invalidArg and invalidOp compile to a plain Exception on every target, so :? ArgumentException and :? InvalidOperationException handlers never catch them, or the library errors raised through them (Array.init -1, List.item, Seq.head on an empty sequence, ...).

Each library already defines these System exceptions, and nullArg already raises ArgumentNullException through FSharp.Core.Operators. This routes invalidArg and invalidOp the same way on JS/TS, Python and Dart. Beam builds its tagged exception map inline, as it does for those exceptions' constructors. Messages don't change.

Rust is unchanged, since it can't type-test BCL exceptions yet.

With the types visible, List.take and Seq.take raise ArgumentException where .NET raises InvalidOperationException, and List.reduce the reverse. I left those for a follow-up.

The full CI passes on my fork: https://github.com/leonitousconforti/Fable/actions/runs/39982396840. I also ran the JS output under JavaScriptCore: a repro matches .NET for all of the above, and a library's JS suite that catches these by type passes in full.

…nd invalidOp

`invalidArg` and `invalidOp` threw a plain `Exception`, so
`:? ArgumentException` and `:? InvalidOperationException` handlers never
caught them, nor the library errors raised through them. They now go
through `FSharp.Core.Operators` like `nullArg`, raising the `System`
exceptions the libraries already define. Beam tags its exception maps
inline, as it does for those exceptions' constructors.

This branch has not been deployed

No deployments
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.

1 participant