Document Parser and Instance exception behavior - #146
Conversation
andreaTP
left a comment
There was a problem hiding this comment.
Thanks for putting this up!
Left a nitpick.
it is expected / acceptable
I don't want to leave the wrong impression here.
I'm not trying to downplay, but that's kind of the same behavior across several runtimes I have experimented with.
If you have a better alternative, please let's discuss.
| * | ||
| * <p>If building fails, for example due to invalid or unsupported Wasm code, an exception | ||
| * is thrown. In many cases that exception will be a {@link WasmEngineException} or a | ||
| * subclass of it, but callers should be prepared to handle any kind of {@code |
There was a problem hiding this comment.
I would not stress:
but callers should be prepared to handle any kind of {@code* RuntimeException}
isn't it "always true"?
There was a problem hiding this comment.
No, at least not for all libraries. Many (including the JDK) often try to document all exceptions with @throws / throws, and any undeclared exception is considered a bug. That helps the user to determine which exact exceptions to catch, instead of wildcard catch (Exception) or catch (RuntimeException).
Though I understand that ensuring this for Endive might not be easily possible, due to Wasm being quite complex and there being lots of potential error cases (and detecting all might cause overhead).
So I think it would be useful to explicitly point out to users that just catching WasmEngineException is probably not enough.
Or the question is, is a non-WasmEngineException only expected for malformed code?
(But currently even Endive itself has explicit throw new RuntimeException; see #151).
Not really. As mentioned in #98 I see mainly two alternatives:
But neither of these is really ideal. |
andreaTP
left a comment
There was a problem hiding this comment.
ok, I think we can move forward with this, is anyways an improvement.
Thanks for the discussion!
Follow-up for #118 (comment) / #118 (comment)
Resolves #98
(as you mentioned there, it is expected / acceptable that non-
WasmEngineExceptionare thrown)I hope the wording is ok like this, but please let me know if you want it to be changed.