Conversation
|
Updated 4:17 AM PT - Apr 7th, 2025
✅ @dylan-conway, your commit ff08e62146ee462da9c6a8c47983e6314e3931b6 passed in 🧪 try this PR locally: bunx bun-pr 18825 |
190n
left a comment
190n
left a comment
There was a problem hiding this comment.
Some style feedback on the tests, but nothing major
| int32_t *data = new int32_t; | ||
| *data = 0; |
There was a problem hiding this comment.
doesn't seem necessary if execute_for_null_complete isn't using it
| }; | ||
|
|
||
| void execute_for_cancel(napi_env env, void *data) { | ||
| // nothing |
There was a problem hiding this comment.
this should never be called, right? we could have it print something or crash so the test can check if it was called
There was a problem hiding this comment.
It won't be called, but it's needed because napi_create_async_work validates it and return napi_invalid_arg if it's null
There was a problem hiding this comment.
I wasn't suggesting to pass null, I was suggesting to make the function print something so we can make sure it's not erroneously called
There was a problem hiding this comment.
oops misread, yeah that's a good idea
| nativeTests.test_napi_async_work_execute_null_check = () => { | ||
| const res = nativeTests.create_async_work_with_null_execute(); | ||
| if (res) { | ||
| console.log("success!"); | ||
| } else { | ||
| console.log("failure!"); | ||
| } | ||
| }; | ||
|
|
||
| nativeTests.test_napi_async_work_complete_null_check = async () => { | ||
| nativeTests.create_async_work_with_null_complete(); | ||
| await gcUntil(() => true); | ||
| }; |
There was a problem hiding this comment.
These two could probably just be called directly by napi.test.ts (and have the native code print out whether everything's okay) instead of having new code in module.js
| if (str) |ptr| { | ||
| if (NAPI_AUTO_LENGTH == length) { | ||
| break :brk bun.sliceTo(@as([*:0]const u8, @ptrCast(str)), 0); | ||
| break :brk bun.sliceTo(@as([*:0]const u8, @ptrCast(ptr)), 0); |
There was a problem hiding this comment.
surprised this ptrCast (in the former code where the input was an optional) is legal
4 participants
What does this PR do?
Fixes four bugs:
executecallback (returnnapi_invalid_argif null).completecallback after work is done (do nothing if null).napi_cancel_async_work. Returnsnapi_cancelledthroughcomplete.napi_async_workthrough any function other thannapi_delete_async_work.How did you verify your code works?
Added a test for null
execute, nullcomplete, and receivingnapi_cancelledafternapi_cancel_async_work