Open LinqLover opened 5 months ago
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:
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:
vm.plugins.javascript.js
, use err instanceof Error ? err.message : String(err)
to assure a meaningful error message.vm.plugins.javascript.js
, collect and return true throw
values (e.g., Exception
objects or other values). In JSBridge
, wrap the result of primGetError
into a JSException
and signal that rather a generic error:
. In addition, support non-Errors
in JSException
(see below).Discussion:
JSException
: Signaling specific subclasses of Error
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:
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 access instanceof
through the plugin and the equivalent of myString.property
in JSBridge does not answer undefined
but signals an error)).primitveGetErrorObject
to the existing one and use both when constructing a JSException
object.Discussion:
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
.
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.
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.