Skip to content

fix(can.cc): use napi_make_callback to restore microtask draining between onMessage callbacks#161

Merged
sebi2k1 merged 1 commit into
masterfrom
fix/napi-make-callback-microtask
May 27, 2026
Merged

fix(can.cc): use napi_make_callback to restore microtask draining between onMessage callbacks#161
sebi2k1 merged 1 commit into
masterfrom
fix/napi-make-callback-microtask

Conversation

@sebi2k1
Copy link
Copy Markdown
Owner

@sebi2k1 sebi2k1 commented May 25, 2026

Problem

Users of socketcan 4.1.0 (the N-API migration) see two symptoms compared to 4.0.7 (NaN) when using adapters like ioBroker.e3oncan on a busy CAN bus:

  1. Constant "Bad frame" warnings during normal operation
  2. "Evaluation of messages overloaded. Counter=400" — the adapter's async processing queue fills faster than it drains

Both symptoms disappear immediately on downgrade to 4.0.7.

Root cause

The NaN implementation called Nan::Callback::Call() which routes through node::MakeCallback(). That function wraps every callback in a node::InternalCallbackScope whose destructor runs a microtask checkpoint (drains Promises and process.nextTick) and fires before/after async hooks.

The N-API migration replaced this with fn.Call()napi_call_function(), which skips both steps.

On a busy bus, the uv_async_t batch can contain up to MAX_FRAMES_PER_ASYNC_EVENT = 100 frames. Without a microtask checkpoint between callbacks, all 100 fire back-to-back. Any async work the adapter queues inside onMessage (e.g. a resolved Promise that dequeues the next item from a processing queue) accumulates as 100 pending microtasks instead of being interleaved one-per-frame. The processing queue overflows → "overloaded" warning → missed/malformed deliveries → "bad frame".

Fix

Replace fn.Call() with napi_make_callback() in async_receiver_ready() — the code path that is called from a uv_async_t callback (i.e. outside a running JS call, exactly the case napi_make_callback is designed for). This restores node::MakeCallback semantics.

Secondary fix: replace the silent env.IsExceptionPending() + break pattern with napi_get_and_clear_last_exception + napi_fatal_exception, matching what Nan::FatalException / Nan::TryCatch did. Without this, a single thrown exception leaves the env in a corrupted state where all subsequent napi_* calls silently fail, delivering empty {} objects to listeners (the adapter sees them as "bad frames").

Test

test/test-callback_ordering.js — sends 30 frames in one batch and verifies that the Promise / process.nextTick scheduled by callback N has resolved before callback N+1 fires. This test would fail deterministically against the old fn.Call() code.

Checklist

  • Reproduces the reported regression scenario
  • New test added that would fail without the fix
  • CHANGELOG updated
  • CI passes (Node.js 22 + 24, Ubuntu)

🤖 Generated with Claude Code

…sage callbacks

The N-API migration replaced Nan::Callback::Call() with fn.Call()
(napi_call_function).  The NaN version used node::MakeCallback internally,
which runs a microtask checkpoint and fires async hooks after each JS
callback.  napi_call_function does neither.

On a busy bus the entire batch of up to MAX_FRAMES_PER_ASYNC_EVENT frames
fired back-to-back with no opportunity for Promise continuations or
process.nextTick callbacks to run.  Adapters that queue processing work
asynchronously inside onMessage (e.g. ioBroker.e3oncan) saw their internal
queue fill faster than it drained, producing "Evaluation of messages
overloaded. Counter=400" warnings and cascading "Bad frame" errors.

Fix: replace fn.Call() with napi_make_callback(), using a napi_async_context
created in Start() and destroyed in async_channel_stopped().  This restores
the node::MakeCallback semantics from the NaN era.

Also replace the silent env.IsExceptionPending()+break pattern with
napi_get_and_clear_last_exception + napi_fatal_exception, matching what
Nan::FatalException / Nan::TryCatch did: exceptions from callbacks are
forwarded to Node's uncaughtException handler rather than left pending in the
env (where they would silently corrupt subsequent napi calls, delivering empty
{} objects to listeners and triggering further "bad frame" warnings).

Add test/test-callback_ordering.js to verify that a microtask (Promise and
process.nextTick) scheduled in callback N resolves before callback N+1 fires.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sebi2k1 sebi2k1 changed the base branch from migrate/nan-to-node-addon-api to master May 25, 2026 15:46
@sebi2k1 sebi2k1 merged commit 59142ff into master May 27, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Behavioral regression in N-API port: Bad frame warnings and message overload

1 participant