Skip to content

Commit 386c3ef

Browse files
committed
Address remaining AI-reviewer concerns on PR #166
- `Console.cpp`: clear any pending N-API exception after the best-effort stack capture. N-API operations like `Object::Get` can leave a pending JS exception on `env` independently of throwing a C++ exception (e.g., a property accessor that throws). Without clearing, returning to the polyfill's wrapper function would surface the pending exception and `console.*` would itself throw on the JS side -- defeating the "side-effect free" contract. - `Console.h`: document the lifetime of `CallbackT`'s `const char*` message (valid only for the duration of the callback; copy to retain). Strengthen the `CaptureCurrentJsStack` doc to explicitly state capture is best-effort, may return empty even from a valid JS context, and hosts must check before using. - `Readme.md`: note that the helper is best-effort and may return empty. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent baadc77 commit 386c3ef

3 files changed

Lines changed: 22 additions & 3 deletions

File tree

Polyfills/Console/Include/Babylon/Polyfills/Console.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,13 @@ namespace Babylon::Polyfills::Console
1717
Error,
1818
};
1919

20+
/**
21+
* Callback invoked for each `console.log` / `console.warn` / `console.error` call.
22+
*
23+
* The `const char*` message is the formatted message produced by joining the call arguments
24+
* (like browsers do). Its storage is only valid for the duration of the callback -- copy if
25+
* you need to retain it. The `LogLevel` indicates which `console.*` method was invoked.
26+
*/
2027
using CallbackT = std::function<void BABYLON_API (const char*, LogLevel)>;
2128

2229
void BABYLON_API Initialize(Napi::Env env, CallbackT callback);
@@ -29,8 +36,10 @@ namespace Babylon::Polyfills::Console
2936
* `console.*` call. The polyfill's own shim frame(s) are skipped, so the top frame is the
3037
* user's call site.
3138
*
32-
* Format is engine-defined opaque text -- treat as diagnostic-only, do not parse. Returns an
33-
* empty string if no JS context is active or if stack capture fails for any reason.
39+
* Format is engine-defined opaque text -- treat as diagnostic-only, do not parse. Capture is
40+
* **best-effort**: returns an empty string if no JS context is active, the engine doesn't
41+
* expose a `stack` property on `Error`, or any internal operation fails. Hosts must check
42+
* for an empty result before using the value.
3443
*
3544
* Cost is non-trivial (currently implemented via `new Error().stack` under the hood); the
3645
* caller decides per-message whether to invoke. Hosts that want stacks only on error

Polyfills/Console/Readme.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ Babylon::Polyfills::Console::Initialize(env, [](const char* message, auto) {
1414
});
1515
```
1616
17-
Inside the callback you can optionally capture the JavaScript callstack at the originating `console.*` call site via `Babylon::Polyfills::Console::CaptureCurrentJsStack(env)`. The polyfill's own shim frames are skipped, so the top frame is the user's call site. The capture is opt-in per-call -- hosts that don't need stacks pay nothing; hosts that want them on a specific level can branch on the `LogLevel` argument:
17+
Inside the callback you can optionally capture the JavaScript callstack at the originating `console.*` call site via `Babylon::Polyfills::Console::CaptureCurrentJsStack(env)`. The polyfill's own shim frames are skipped, so the top frame is the user's call site. Capture is best-effort -- the returned string can be empty (no JS context active, engine doesn't expose `Error.stack`, etc.), so always check before using. The capture is opt-in per-call -- hosts that don't need stacks pay nothing; hosts that want them on a specific level can branch on the `LogLevel` argument:
1818
```c++
1919
Babylon::Polyfills::Console::Initialize(env, [env](const char* message, auto level) {
2020
fprintf(stdout, "%s", message);

Polyfills/Console/Source/Console.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,16 @@ namespace Babylon::Polyfills::Console
165165
catch (...)
166166
{
167167
}
168+
169+
// N-API operations can leave a pending JS exception on `env` independently of throwing a
170+
// C++ exception (e.g., `Object::Get` on a property accessor that throws); returning from
171+
// the callback with a pending exception would cause `console.*` itself to throw on the
172+
// JS side. Clear it so stack capture is truly side-effect free.
173+
if (env.IsExceptionPending())
174+
{
175+
(void)env.GetAndClearPendingException();
176+
}
177+
168178
return stack;
169179
}
170180
}

0 commit comments

Comments
 (0)