ghcjs / ghcjs-dom

Make Document Object Model (DOM) apps that run in any browser and natively using WebKitGtk
74 stars 41 forks source link

ghcjs-dom-jsffi JS exceptions can not be caught in Haskell #78

Closed eskimor closed 6 years ago

eskimor commented 7 years ago

According to the ghcjfs-ffi this should be easily fixable by using a safe foreign import as opposed to an unsafe one.

Is the fix really this easy? Are there any good reasons why everything gets imported unsafe?

Note: with jsaddle and ghc JS exceptions can be handled just fine.

hamishmack commented 7 years ago

Is the fix really this easy? Are there any good reasons why everything gets imported unsafe?

My understanding is that it results in larger (probably slower) JavaScript as it has to wrap each call with try/catch. What functions in particular do we need to handle exceptions from? If we can identify them somehow in the IDL files we can change the generator to use safe for them.

eskimor commented 7 years ago

A function where it was already a problem for me was RTCPeerConnection.close which throws if the connection was already closed. I will checkout the IDL files, whether they say anything about exceptions.

Otherwise I will try to write some benchmark to check for performance implications.

hamishmack commented 7 years ago

Are you sure it was close that was throwing? The IDL file does not look like it will be helpful https://github.com/WebKit/webkit/blob/master/Source/WebCore/Modules/mediastream/RTCPeerConnection.idl#L97, but interestingly the function setConfiguration in the same interface has a [MayThrowException] attribute.

eskimor commented 7 years ago

I am afraid so, at least on Chrome. But as I just checked, it does not seem to be a standard conform behavior! According to the spec, step 2 says:

If connection's [[isClosed]] slot is true, abort these steps.

so it should not throw, but just do nothing. I will file a bug in Chromium about that!

eskimor commented 7 years ago

Ok, I just rechecked:

uncaught exception in Haskell thread: InvalidStateError: Failed to execute 'close' on 'RTCPeerConnection': The RTCPeerConnection's signalingState is 'closed'.
rts-1a9e85f….js:7062 Error: Failed to execute 'close' on 'RTCPeerConnection': The RTCPeerConnection's signalingState is 'closed'.
    at h$$ghcjszmdomzmjsffizm0zi9zi2zi0zm8wrBcFMn4GK4ObK9AtPw9ZCGHCJSziDOMziJSFFIziGeneratedziRTCPeerConnection_je (out-9e25d10….js:333911)
    at h$runThreadSlice (rts-1a9e85f….js:8017)
    at h$runThreadSliceCatch (rts-1a9e85f….js:7968)
    at h$mainLoop (rts-1a9e85f….js:7963)
    at rts-1a9e85f….js:2460
    at runIfPresent (rts-1a9e85f….js:2478)
    at onGlobalMessage (rts-1a9e85f….js:2512)
VM717:22 

I am going to file a bug report.

eskimor commented 7 years ago

https://bugs.chromium.org/p/chromium/issues/detail?id=757254&q=RTCPeerConnection.close&colspec=ID%20Pri%20M%20Stars%20ReleaseBlock%20Component%20Status%20Owner%20Summary%20OS%20Modified

eskimor commented 7 years ago

Bug got fixed. Ok, I think the way to go forward would be to import functions marked throwing in the idl files as safe and all others as unsafe. My colleague and I will try to provide a PR soon.