SqueakJS
SqueakJS copied to clipboard
JSBridge-Core-ct.11: Fix superclass of JSException
One should (always) never subclass from Exception itself, but most of the time from Error or Notification or one of its subclasses. Exception is an abstract class, and the previous implementation led to SubclassResponsibility errors from Exception>>defaultAction when signaling a JSException. Since JavaScript errors do not have Smalltalk''s coroutine capabilities, subclassing from Error seems a perfect match in this case.
Interesting, thanks! Could you please also update JSBridge.st which is used by pre-Monticello images?
And while you're at it, I think we should update JSObjectProxy>>await:
so it works in old images without exception handling (e.g. Dan's mini.image with our default JSBridge demo, which BTW is why the primitives are declared the way they are). I have not tested it but something like this, maybe?
...
isError ifTrue: [
JSException superclass
ifNil: [self error: result]
ifNotNil: [JSException error: result]].
In Squeak/Smalltalk, one can implement dynamic scope through Exception
. One example is CurrentReadOnlySourceFiles
.
One should (always) never subclass from Exception itself [...]
...if one means to model exceptions in a somewhat traditional sense. Yes. In Squeak, there are "notifications in disguise" (i.e., resumable exceptions) such as WebAuthRequired
and MCNoChangesException
. There are also "errors in disguise" (i.e., non-resumable exceptions) such as TestFailure
. And some special things such as Halt
and UnhandledError
, the former almost an error but resumable. So much fun. :-)
@codefrau Sorry for the delay!
Yes, your fix works in the mini image. I prefer to add this to the beginning of JSException>>error:
:
self class superclass ifNil: [^ nil error: error asString].
Other than that, I think I found a few more issues:
SqueakJS does not support custom error values
The ECMA specs do not require values that are passed to throw
^1 or Promise.reject
^2 to be Error
instances, but allows arbitrary values such as 42
or "hi"
as well. E.g., throw 42;
or new Promise((resolve, reject) => reject(42));
are valid though not recommended. How can we fix that?
throw
Example:
JS eval: 'throw 42'. --> ⚡ Error: undefined "expected: Error: 42"
Possible alternative fixes:
-
Fix error decoding in the VM plugin: In
vm.plugins.javascript.js
, useerr instanceof Error ? err.message : String(err)
to assure a meaningful error message. -
Delegate error decoding from VM to image: In
vm.plugins.javascript.js
, collect and return truethrow
values (e.g.,Exception
objects or other values). InJSBridge
, wrap the result ofprimGetError
into aJSException
and signal that rather a genericerror:
. In addition, support non-Errors
inJSException
(see below).
Discussion:
- Pro 1:
- Smallest change.
- Maintains backward compatibility of the plugin interface. (If compatibility matters, we could also offer a new primitive in addition to the old one.)
- Pro 2:
- Stops omitting information and allows for introspection of any thrown values in the image.
- Consistent use of
JSException
: Signaling specific subclasses ofError
is commonly better style in Squeak and simplifies nunaced exception handling.
Promise.reject()
Example:
JS eval: 'new Promise((resolve, reject) => reject(42))'. --> ⚡ Error: undefined "expected: Error: 42"
Possible fixes:
-
Decode errors in the image: In
JSException
, add something like((JS eval: '(e) => e instanceof Error ? e.message : String(e)') call: undefined value: error) asString
(all the indirection because we cannot accessinstanceof
through the plugin and the equivalent ofmyString.property
in JSBridge does not answerundefined
but signals an error)). -
Decode errors in the VM and deliver them separately into the image: As suggested above, maybe add a new
primitveGetErrorObject
to the existing one and use both when constructing aJSException
object.
Discussion:
- Pro 1: Keeps plugin interface minimal.
- Pro 2: Avoids handshakes between VM and image (I assume faster). In line with the backward-compatible proposal from above.
tl;dr my preferred solution would be to add a new primitiveGetErrorObject
to the plugin to provide both the raw error value/string/whatever and a string/message version separately to the image; use it in JSException
, and reuse the JSException
route in JSObjectProxy
.
Default SqueakJS GUI does not support promises
Set up a fresh trunk image, installed JSBridge-Core
, and opened it in https://squeak.js.org/run. Example:
JS await: (JS eval: 'new Promise(resolve => resolve(42))')
Similarly, JS eval: 'await 42'
fails due to lack of asynchronous context.
Times out forever. /cc'ing @tom95 & @leogeier who made a similar observation.
Sorry for the wall of text. Looking forward to your opinions! Yes, I will patch JSBridge.st
as well.
I like your analysis! I agree we should wrap thrown objects in JSException
rather than convert them to strings. I just added a primitiveGetErrorObject
to the VM (see ab9b81820bec4e1b7edc0f5bb7e651b414879bfd).
For historical context: JSException
got added by Bill Burdick (WRB) to my initial JSBridge
implementation where I only used self error: 'string'
. However, we did not change over everything to exceptions. Now is a good time to do so.
Re: JSException>>error:
for pre-Exception images – your suggestion sounds reasonable. It would be even nicer if we could send error:
not to nil
but to the JSObjectProxy
that encountered the error, for a nicer debugging experience. Something like JSException error: xyz in: self
and store both the proxy and the error (or whatever was thrown) in the exception.
Re: "SqueakJS GUI does not support promises" – works on my machine 😉
In the mini image, JS await: (JS eval: 'new Promise(resolve => resolve(42))')
works as expected, answering 42
. There must be something different in the trunk image (I haven't tried that).
I'm not worried about JS eval: 'await 42'
failing because that is the case for a plain JS eval('await 42')
too. It's precisely why we have await:
.
@codefrau Sorry for not replying earlier! While trying to debug JS await:
, I stumbled upon some other primitive issues in SqueakJS and tried to fix them first (see my recent trunk versions for Kernel/KernelTests and my PRs in this repo). I think most of them should be fixed now, and I'm looking forward to your reviews 🙂, but half a week is gone mainly for that and I guess I should return to my actual work before I continue exploring promises in SqueakJS ... so I will have to put this on ice for now and return later, hopefully soon!
Just some very brief replies below:
I like your analysis! I agree we should wrap thrown objects in
JSException
rather than convert them to strings. I just added aprimitiveGetErrorObject
to the VM (see ab9b818).
Thanks! Looking forward to integrate them on the image side soon if you do not preempt me. :-)
For historical context:
JSException
got added by Bill Burdick (WRB) to my initialJSBridge
implementation where I only usedself error: 'string'
. However, we did not change over everything to exceptions. Now is a good time to do so.Re:
JSException>>error:
for pre-Exception images – your suggestion sounds reasonable. It would be even nicer if we could senderror:
not tonil
but to theJSObjectProxy
that encountered the error, for a nicer debugging experience. Something likeJSException error: xyz in: self
and store both the proxy and the error (or whatever was thrown) in the exception.
I like that!
Re: "SqueakJS GUI does not support promises" – works on my machine 😉 In the mini image,
JS await: (JS eval: 'new Promise(resolve => resolve(42))')
works as expected, answering42
. There must be something different in the trunk image (I haven't tried that).
It works on the demo page for me too, but not on https://squeak.js.org/run. Do they share the same GUI code?
I'm not worried about
JS eval: 'await 42'
failing because that is the case for a plain JSeval('await 42')
too. It's precisely why we haveawait:
.
It works on the demo page for me too, but not on https://squeak.js.org/run. Do they share the same GUI code?
Yes, the GUI is created by createSqueakDisplay which is shared by all pages. Only when running on Lively there is different GUI code.
After running the demo, you should find "squeakjs.image" on the /run page. Clicking that works just fine for me.