Bug 215877 – [GLIB] No way to determine if an exception has been thrown calling jsc_value_object_invoke_methodv

• Status: NEW
• Component: JavaScriptCore

[Michael Gratton]

The JSC method `jsc_value_object_invoke_methodv` returns neither a GError or a JSCValue when the method being invoked is an exception, hence it is not possible to determine if the method invocation was successful or not.

This is a regression from the old JS API, where JSObjectCallAsFunction did.

This would be useful in Geary for QA since we can't use unit tests to detect failing invocations.

[Michael Gratton]

"method being invoked raises an exception"

[Michael Gratton]

So I've just found `jsc_context_get_exception` and `jsc_context_clear_exception`, which helps (just)! There are some problems with this however:

 1. It is racy between invocations
 2. There's no documentation about when it's valid to call either method, if the latter needs to be called and if so, when
 3. There is no documentation to suggest this is actually useful for the use case above
 4. No members are ever set when this object is non-null due to an exception being thrown after call to `jsc_value_object_invoke_methodv` - name, source-uri, etc are always null, line and column numbers are always 0 (they're not even -1, since these are uints)

So aside from the fixes needed for (2)-(4), consider this a feature request for an API that doesn't have the limitations of (1). Thanks!

[Carlos Garcia Campos]

Exceptions are handled a bit different in the GLib JSC API compared to other GLib based APIs. JavaScript APIs can change whether they can throw exceptions or not, without affecting the API, but in C, adding or removing GError parameter would be an API change. So, exceptions are handled by the JSCContext that keeps a reference to the latest exception being thrown. This is similar to other APIS like cairo or OpenGL. To handle the exceptions you can either check jsc_context_get_exception() after an API call and handle it by clearing it, or you can push your own handler with jsc_context_push_exception_handler(). Until the handler is popped with jsc_context_pop_exception_handler(), the callback given to the push will be called every time an expcetion is handled. Inside the callback you can handle the exception or throw it by calling jsc_context_throw_exception().

So, if after calling jsc_value_object_invoke_methodv(), and the method invocation causes and exception, jsc_context_get_exception() returns null, the this is indeed a bug.

[Carlos Garcia Campos]

(In reply to Michael Gratton from comment #2)
> So I've just found `jsc_context_get_exception` and
> `jsc_context_clear_exception`, which helps (just)! There are some problems
> with this however:
> 
>  1. It is racy between invocations

What's racy?

>  2. There's no documentation about when it's valid to call either method, if
> the latter needs to be called and if so, when

You can check if there's an exception being raised in the context, and you can clear ii to "handle" it, if it's not fatal, to ensure new exceptions are thrown.

>  3. There is no documentation to suggest this is actually useful for the use
> case above

What use case exactly?

>  4. No members are ever set when this object is non-null due to an exception
> being thrown after call to `jsc_value_object_invoke_methodv` - name,
> source-uri, etc are always null, line and column numbers are always 0
> (they're not even -1, since these are uints)

What's the script name and row/column of a method called by jsc_value_object_invoke_methodv?

> So aside from the fixes needed for (2)-(4), consider this a feature request
> for an API that doesn't have the limitations of (1). Thanks!

I don't understand the limitations, could you elaborate? Maybe a test case would help me to understand the issue.

[Michael Gratton]

(In reply to Carlos Garcia Campos from comment #4)
> (In reply to Michael Gratton from comment #2)
> > So I've just found `jsc_context_get_exception` and
> > `jsc_context_clear_exception`, which helps (just)! There are some problems
> > with this however:
> > 
> >  1. It is racy between invocations
> 
> What's racy?

1. Call jsc_value_object_invoke_methodv in an async or threaded context
3. Call jsc_value_object_invoke_methodv in a different async/threaded context
4. return to the call from (1)
5. call jsc_context_get_exception

If non-null, which call does the exception apply to?

If the answer here is always "call `jsc_context_get_exception` immediately after invoking a JS callable so the exception isn't overwritten" then it would be nice if the API did that automatically so that people that use the API don't have to know or remember to do so.

If the answer is "This isn't possible" then that's not documented (either).

> >  2. There's no documentation about when it's valid to call either method, if
> > the latter needs to be called and if so, when
> 
> You can check if there's an exception being raised in the context, and you
> can clear ii to "handle" it, if it's not fatal, to ensure new exceptions are
> thrown.

Great, but that's not documented, hence this bug.

> 
> >  3. There is no documentation to suggest this is actually useful for the use
> > case above
> 
> What use case exactly?

Determining if an exception has been called after calling `jsc_value_object_invoke_methodv` (or some other way to invoke a JS callable). I only found it much later because I was looking at something else.

> >  4. No members are ever set when this object is non-null due to an exception
> > being thrown after call to `jsc_value_object_invoke_methodv` - name,
> > source-uri, etc are always null, line and column numbers are always 0
> > (they're not even -1, since these are uints)
> 
> What's the script name and row/column of a method called by
> jsc_value_object_invoke_methodv?

In the case of an exception being raised by a method called by `jsc_value_object_invoke_methodv` that is defined in a JavaScript source file, the script name is the name of the source file and the row/column is the line number and column number of the throw statement, presumably?

> > So aside from the fixes needed for (2)-(4), consider this a feature request
> > for an API that doesn't have the limitations of (1). Thanks!
> 
> I don't understand the limitations, could you elaborate? Maybe a test case
> would help me to understand the issue.

These are all issues of API documentation and ergonomics, so I guess I am the test case. :) More specifically, I've been porting Geary to use the UserMessage API and have been reporting bugs for things that are missing based on that experience.

[Carlos Garcia Campos]

(In reply to Michael Gratton from comment #5)
> (In reply to Carlos Garcia Campos from comment #4)
> > (In reply to Michael Gratton from comment #2)
> > > So I've just found `jsc_context_get_exception` and
> > > `jsc_context_clear_exception`, which helps (just)! There are some problems
> > > with this however:
> > > 
> > >  1. It is racy between invocations
> > 
> > What's racy?
> 
> 1. Call jsc_value_object_invoke_methodv in an async or threaded context
> 3. Call jsc_value_object_invoke_methodv in a different async/threaded context
> 4. return to the call from (1)
> 5. call jsc_context_get_exception
> 
> If non-null, which call does the exception apply to?

I think JavaScript always works synchronously in the main thread, so you should not use the jsc api from a secondary thread. This applies to a JSCContext created in the WebKit web process. The API itself can be used concurrently.

The answer would be, the last one that thrown the exception. JSC context always has the last uncaught exception.

> If the answer here is always "call `jsc_context_get_exception` immediately
> after invoking a JS callable so the exception isn't overwritten" then it
> would be nice if the API did that automatically so that people that use the
> API don't have to know or remember to do so.

What can we do automatically? When an exception is thrown and not handled, it's set in the context. You can push your own handler to ensure you catch the exception as soon as it's thrown.

I don't see much difference between

invoke(&error);
if (error)
 
and

invoke();
if (contxt_get_exception())


> If the answer is "This isn't possible" then that's not documented (either).
> 
> > >  2. There's no documentation about when it's valid to call either method, if
> > > the latter needs to be called and if so, when
> > 
> > You can check if there's an exception being raised in the context, and you
> > can clear ii to "handle" it, if it's not fatal, to ensure new exceptions are
> > thrown.
> 
> Great, but that's not documented, hence this bug.
> 
> > 
> > >  3. There is no documentation to suggest this is actually useful for the use
> > > case above
> > 
> > What use case exactly?
> 
> Determining if an exception has been called after calling
> `jsc_value_object_invoke_methodv` (or some other way to invoke a JS
> callable). I only found it much later because I was looking at something
> else.

You can either call get_exception() or push tour handler.

> > >  4. No members are ever set when this object is non-null due to an exception
> > > being thrown after call to `jsc_value_object_invoke_methodv` - name,
> > > source-uri, etc are always null, line and column numbers are always 0
> > > (they're not even -1, since these are uints)
> > 
> > What's the script name and row/column of a method called by
> > jsc_value_object_invoke_methodv?
> 
> In the case of an exception being raised by a method called by
> `jsc_value_object_invoke_methodv` that is defined in a JavaScript source
> file, the script name is the name of the source file and the row/column is
> the line number and column number of the throw statement, presumably?

How are you evaluating the JavaScript source file in the context?

> > > So aside from the fixes needed for (2)-(4), consider this a feature request
> > > for an API that doesn't have the limitations of (1). Thanks!
> > 
> > I don't understand the limitations, could you elaborate? Maybe a test case
> > would help me to understand the issue.
> 
> These are all issues of API documentation and ergonomics, so I guess I am
> the test case. :) More specifically, I've been porting Geary to use the
> UserMessage API and have been reporting bugs for things that are missing
> based on that experience.

I supposed it, I'm happy to help to get geary ported to use the new JSC API.

[Radar WebKit Bug Importer]

<rdar://problem/68247528>

[Adrian Perez]

Michael, did the suggestions from the previous comments work for
you when using the JSC API? If so, I believe we could close this
bug :)