Fix handling of termination exceptions. #3029
Conversation
Exceptions caught in v8::TryCatch scopes need to be rethrown in order for the next try/catch handler down the stack to be notified of them. In some code paths, we rethrow the native jsg::JsExceptionThrown exception but don't rethrow the v8 exception using v8::TryCatch::ReThrow(). This commit fixes one such code path which resulted in IoContext::runImpl's jsg::JsExceptionThrown's catch block finding that the corresponding v8::TryCatch object hadn't been notified of the termination (and didn't contain an exception).
|
Note that this change ensures that we now hit this conditional branch. I wanted to generally prompt a discussion about how we want to handle various types of V8 exception, though. At the moment, it seems like our combination of Further context: we first started hitting this when we added this catch clause which allowed termination exceptions to bubble up to the try/catch in |
| // This may happen in particular when the JsExceptionThrows was the result of | ||
| // TerminateException() but V8 has since cleared the terminate flag because all | ||
| // This may happen in particular when the JsExceptionThrown was the result of | ||
| // TerminateExecution() but V8 has since cleared the terminate flag because all |
There was a problem hiding this comment.
@jasnell what exactly is this comment referring to? Is it referencing this V8 logic here? I couldn't easily figure out under exactly what circumstances CallDepthIsZero() would be true since comments seemed to suggest that it pertains to "V8 API" call depth rather than JS call depth.
Exceptions caught in
v8::TryCatchscopes need to be rethrown in order for the next try/catch handler down the stack to be notified of them. In some code paths, we rethrow the nativejsg::JsExceptionThrownexception but don't rethrow the v8 exception usingv8::TryCatch::ReThrow().This commit fixes one such code path which resulted in
IoContext::runImpl'sjsg::JsExceptionThrown's catch block finding that the correspondingv8::TryCatchobject hadn't been notified of the termination (and didn't contain an exception).