Hi, I would like to ask about an exception handling policy in JSC C API implementation. After checking the code I found a common pattern: (A) if (exec->hadException()) { if (exception) *exception = toRef(exec, exec->exception()); exec->clearException(); } Which means that exception handler value is set only when an exception occurs. Another approach that I found is in JSValueCreateJSONString: (B) if (exception) *exception = 0; if (exec->hadException()) { if (exception) *exception = toRef(exec, exec->exception()); exec->clearException(); } That code always set exception handler. Advantage of this solution (B) is that after call, an api user can check if actually an exception occured by checking value of the exception handler (it was possible with the first approach (A) by initialize exception handler to null, but it is rather a side effect). The B pattern is not optimal, because the exception handler may be reinitialized. I believe that an exception handler should be used for simple error checking; - there is not null exception value => it means that returned value is not valid, because of an error. - there is null exception => everything is ok. What do you think about code C? (C) if (exception) { if (exec->hadException()) { *exception = toRef(exec, exec->exception()); exec->clearException(); } else *exception = 0; } else exec->clearException(); In this case exception handler is always set correctly. Would it make sense to change the behavior? The change unifies interface and is binary compatible. It shouldn't break a correct code... :-) Cheers, Jędrek
On Jun 1, 2010, at 5:44 AM, Jedrzej Nowacki wrote:
Another approach that I found is in JSValueCreateJSONString:
This is a recently-added function, so it’s not surprising that it mistakenly has different design than the older functions. I think it should be changed to use what you called pattern (A).
Would it make sense to change the behavior? The change unifies interface and is binary compatible. It shouldn't break a correct code.
One effect of making the change is that people who tested only with the newer version of JavaScriptCore might depend on the behavior and so be incompatible with older versions of JavaScriptCore. Geoff Garen and Maciej Stachowiak are two other people I’d like to hear from. Both are on vacation and both will be back next week. -- Darin
On Thursday 3. June 2010 03.03.34 ext Darin Adler wrote:
On Jun 1, 2010, at 5:44 AM, Jedrzej Nowacki wrote:
Would it make sense to change the behavior? The change unifies interface and is binary compatible. It shouldn't break a correct code.
One effect of making the change is that people who tested only with the newer version of JavaScriptCore might depend on the behavior and so be incompatible with older versions of JavaScriptCore. Good point. But does it mean that we can't fix any behavioral issues? This is undocumented area and it could be left like that up to the right moment.
Current behavior could lead to problems if uninitialized or badly initialized exception handler will be provided. Moreover it is not possible to check if a value returned from the JSValueToNumber is correct, unless exception handler is initialized to NULL, which seems wrong to me. I don't have a strong opinion about the problem, I have been bitten by the issue twice. Right now, I know that exception handler should be always initialized to NULL and I can live with that :-).
Geoff Garen and Maciej Stachowiak are two other people I’d like to hear from. Both are on vacation and both will be back next week. Sure :-). I'm open for a discussion.
Cheers, Jędrek
Hi Jędrek.
(C) if (exception) { if (exec->hadException()) { *exception = toRef(exec, exec->exception()); exec->clearException(); } else *exception = 0; } else exec->clearException(); In this case exception handler is always set correctly. Would it make sense to change the behavior?
As you said, this suggestion is slightly less efficient than the current behavior. As Darin said, this change might lead to incorrect behavior if a developer tested against the newest JavaScriptCore but deployed on systems with older JavaScriptCores. Another downside to this change is that a programmer who called two API functions in a row, forgetting to check for an exception between them, will not see the exception at all, since the second call will reset the exception value to 0. I'm not sure that these three downsides are overwhelming. But what's the upside to weigh against them? It's slightly inconvenient to initialize the exception value to 0, but, assuming no exception is thrown, a programmer only has to do that once per block of code: JSValueRef exception = 0; a(..., &exception); b(..., &exception); c(..., &exception); Do you have a use case where you end up initializing the exception value over and over again? Or is your goal to save programmers who forget to initialize exception to 0? Geoff
participants (3)
-
Darin Adler
-
Geoffrey Garen
-
Jedrzej Nowacki