Support FunctionCallback returning an error, and native Exceptions. #195
Conversation
If the error is a Valuer (like an *Exception), it will be thrown as a proper JS error, otherwise it becomes a thrown string.
Codecov Report
@@ Coverage Diff @@
## master #195 +/- ##
==========================================
- Coverage 95.86% 94.62% -1.24%
==========================================
Files 17 18 +1
Lines 580 633 +53
==========================================
+ Hits 556 599 +43
- Misses 15 20 +5
- Partials 9 14 +5
Continue to review full report at Codecov.
|
| ) | ||
|
|
||
| // NewRangeError creates a RangeError. | ||
| func NewRangeError(iso *Isolate, msg string) *Exception { |
There was a problem hiding this comment.
There is already the JSError type which is used to return exceptions from v8go functions. It seems like there should be consistency between the exception value returned by other v8go functions and these functions to construct errors. That should make it easier to propagate exceptions, even without adding a new feature to return errors from function callbacks.
Currently, JSError is converting the underlying JS error object to a string and storing it in the Message field. Instead, it should store the underlying JS error object, so that the error can be propagated. That Message field could be deprecated in favour of a method so that the conversion can be done on-demand in the future. It looks like the stack trace can similarly be obtained on-demand using v8::Exception::GetStackTrace.
The JSError.Location field is derived from the a v8::Message on v8::TryCatch, which may be present without a stack trace for compilation errors. v8::Exception::CreateMessage can be used to obtained a v8::Message from an exception value, but we don't want to automatically create it in a getter method. Since it is a public field, it will need to continue populating on creation for the current code paths, but it could similarly be deprecated in favour of a method to obtain this on-demand for new code paths. This on-demand code path could at least obtain the when the error has a stack trace, even if we don't get it to initially work on this new code path errors with a location but no stack trace.
It seems like this refactoring and the addition of these functions to create exceptions could be split out into a separate PR.
There was a problem hiding this comment.
I've opened this as an issue (#274) for greater visibility on this issue with JSError
There was a problem hiding this comment.
I don't remember exactly why I created a new type for it, but it was probably to avoid having to refactor JSError (avoiding merge conflicts). If there's consensus that's better, that sounds good to me. Thanks for reviewing.
DRAFT: Awaiting #274 to refactor
JSError.Returning an error from a callback feels like the most Go way of supporting exceptions, and it ensures the Go code doesn't continue executing after throwing (as it would if we instead supported
Isolate.ThrowException).This also adds Error constructors, so functions can throw real JS Errors. Note that the new callback interface does allow throwing arbitrary objects, but
*Exceptionis the only built-in object that is both aValueranderror.