Skip to content

Commit 410cf6a

Browse files
authored
src: return bool on object freeze and seal (#991)
These operations can call into JavaScript by using Proxy handlers, hence JavaScript exceptions might pending at the end of the operations.
1 parent 93f1898 commit 410cf6a

File tree

5 files changed

+40
-14
lines changed

5 files changed

+40
-14
lines changed

napi-inl.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,14 +1389,16 @@ inline void Object::AddFinalizer(Finalizer finalizeCallback,
13891389
}
13901390

13911391
#if NAPI_VERSION >= 8
1392-
inline void Object::Freeze() {
1392+
inline bool Object::Freeze() {
13931393
napi_status status = napi_object_freeze(_env, _value);
1394-
NAPI_THROW_IF_FAILED_VOID(_env, status);
1394+
NAPI_THROW_IF_FAILED(_env, status, false);
1395+
return true;
13951396
}
13961397

1397-
inline void Object::Seal() {
1398+
inline bool Object::Seal() {
13981399
napi_status status = napi_object_seal(_env, _value);
1399-
NAPI_THROW_IF_FAILED_VOID(_env, status);
1400+
NAPI_THROW_IF_FAILED(_env, status, false);
1401+
return true;
14001402
}
14011403
#endif // NAPI_VERSION >= 8
14021404

napi.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,8 +773,8 @@ namespace Napi {
773773
T* data,
774774
Hint* finalizeHint);
775775
#if NAPI_VERSION >= 8
776-
void Freeze();
777-
void Seal();
776+
bool Freeze();
777+
bool Seal();
778778
#endif // NAPI_VERSION >= 8
779779
};
780780

test/common/index.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ exports.mustNotCall = function(msg) {
7676
};
7777

7878
exports.runTest = async function(test, buildType) {
79-
buildType = buildType || process.config.target_defaults.default_configuration;
79+
buildType = buildType || process.config.target_defaults.default_configuration || 'Release';
8080

8181
const bindings = [
8282
`../build/${buildType}/binding.node`,
@@ -90,7 +90,7 @@ exports.runTest = async function(test, buildType) {
9090
}
9191

9292
exports.runTestWithBindingPath = async function(test, buildType) {
93-
buildType = buildType || process.config.target_defaults.default_configuration;
93+
buildType = buildType || process.config.target_defaults.default_configuration || 'Release';
9494

9595
const bindings = [
9696
`../build/${buildType}/binding.node`,

test/object/object_freeze_seal.cc

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@
44

55
using namespace Napi;
66

7-
void Freeze(const CallbackInfo& info) {
7+
Value Freeze(const CallbackInfo& info) {
88
Object obj = info[0].As<Object>();
9-
obj.Freeze();
9+
return Boolean::New(info.Env(), obj.Freeze());
1010
}
1111

12-
void Seal(const CallbackInfo& info) {
12+
Value Seal(const CallbackInfo& info) {
1313
Object obj = info[0].As<Object>();
14-
obj.Seal();
14+
return Boolean::New(info.Env(), obj.Seal());
1515
}
1616

1717
Object InitObjectFreezeSeal(Env env) {

test/object/object_freeze_seal.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ module.exports = require('../common').runTest(test);
77
function test(binding) {
88
{
99
const obj = { x: 'a', y: 'b', z: 'c' };
10-
binding.object_freeze_seal.freeze(obj);
10+
assert.strictEqual(binding.object_freeze_seal.freeze(obj), true);
1111
assert.strictEqual(Object.isFrozen(obj), true);
1212
assert.throws(() => {
1313
obj.x = 10;
@@ -20,9 +20,21 @@ function test(binding) {
2020
}, /Cannot delete property 'x' of #<Object>/);
2121
}
2222

23+
{
24+
const obj = new Proxy({ x: 'a', y: 'b', z: 'c' }, {
25+
preventExtensions() {
26+
throw new Error('foo');
27+
},
28+
});
29+
30+
assert.throws(() => {
31+
binding.object_freeze_seal.freeze(obj);
32+
}, /foo/);
33+
}
34+
2335
{
2436
const obj = { x: 'a', y: 'b', z: 'c' };
25-
binding.object_freeze_seal.seal(obj);
37+
assert.strictEqual(binding.object_freeze_seal.seal(obj), true);
2638
assert.strictEqual(Object.isSealed(obj), true);
2739
assert.throws(() => {
2840
obj.w = 'd';
@@ -34,4 +46,16 @@ function test(binding) {
3446
// so this should not throw.
3547
obj.x = 'd';
3648
}
49+
50+
{
51+
const obj = new Proxy({ x: 'a', y: 'b', z: 'c' }, {
52+
preventExtensions() {
53+
throw new Error('foo');
54+
},
55+
});
56+
57+
assert.throws(() => {
58+
binding.object_freeze_seal.seal(obj);
59+
}, /foo/);
60+
}
3761
}

0 commit comments

Comments
 (0)