Try catch 6.2.0 #42

Closed
wants to merge 2 commits into
from

Projects

None yet

3 participants

@gabrielschulhof
Collaborator

This implements and tests the API described in #27 (comment) for V8 as present in Node.js 6.2.0.

gabrielschulhof added some commits Jan 7, 2017
@gabrielschulhof gabrielschulhof API: Implement napi_trycatch aa36f8e
@gabrielschulhof gabrielschulhof Tests: Add test for napi_trycatch
(cherry picked from commit 8d8c17fc3458b70abfd2efcaee97538b49ca3008)
92278fa
@mhdawson

Based on discussion today in the api meeting I'll wait to review until I get back from holidays next week as I believe there may be some changes.

+ struct napi_trycatch_impl *local = (struct napi_trycatch_impl *)trycatch;
+
+ if (!(local->exceptionRetrieved)) {
+ local->impl.ReThrow();
@jasongin
jasongin Jan 19, 2017 Collaborator

Should there be a napi_trycatch_rethrow function, so that code that calls napi_trycatch_exception can decide not to handle the exception after looking at it?

+ }
+}
+
+void napi_trycatch_delete(napi_env e, napi_trycatch trycatch) {
@jasongin
jasongin Jan 19, 2017 Collaborator

There should probably be a Napi::TryCatch RAII helper class in node_api_helpers.h that takes care of automatically deleting the trycatch when it goes out of scope.

+ napi_set_property(env, exports,
+ napi_property_name(env, "allowException"),
+ napi_create_function(env, allowException));
+}
@jasongin
jasongin Jan 19, 2017 Collaborator

Should add 2 (negative) test cases for when no exception is thrown and napi_trycatch_exception is/is not called.

@jasongin
Collaborator

Now I realize this PR is made obsolete by the newer Try catch global persistent exception PR... I think.

@gabrielschulhof
Collaborator

Yeah, actually. It's probably bet to close it.

@gabrielschulhof
Collaborator

Sorry about the confusion!

@jasongin
Collaborator

Should the chakracore port of this be closed also?

@gabrielschulhof
Collaborator
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment