Skip to content

feat(runtime): show the actual error when an async job throws#13

Open
wytzepiet wants to merge 1 commit into
fluttercandies:mainfrom
wytzepiet:feat/job-error-detail
Open

feat(runtime): show the actual error when an async job throws#13
wytzepiet wants to merge 1 commit into
fluttercandies:mainfrom
wytzepiet:feat/job-error-detail

Conversation

@wytzepiet
Copy link
Copy Markdown

@wytzepiet wytzepiet commented May 31, 2026

When a queued job (a promise callback, a setTimeout, etc.) throws, executePendingJob reported just:

Async job raised an exception

The real JavaScript error — its message and stack — was thrown away, so you couldn't tell what actually broke.

Here's why: rquickjs hands back an AsyncJobException whose text is exactly that fixed string. The real error is left on the job's context for the caller to read with Ctx::catch (as its docs note). fjs just wasn't reading it.

This change reads it — the same way the runtime's own idle() already does — and puts it in the error. So now you get the real thing:

Async job raised an exception: Error: Maximum call stack size exceeded
    at render (app.js:42)
    ...

Reading the error also clears it, which fixes a small side bug: the old code left the exception pending, so it could show up again on the next executePendingJob call.

The method signature doesn't change — only the error text gets better.

🤖 Generated with Claude Code

JsAsyncRuntime::execute_pending_job mapped rquickjs's AsyncJobException
straight through anyhow, but that type's Display is the opaque
"Async job raised an exception" — the actual JS error and stack are left
on the offending context for the caller to recover (per its docs) and
were thrown away. A host pumping the job queue (e.g. draining microtasks
after a bridge call) only saw that opaque string plus a Rust backtrace,
which is useless for diagnosing a JS-level failure such as an unhandled
promise rejection.

Recover the pending exception from the job's context (the same way the
runtime's own idle() does) and include its message/stack in the returned
error, so the host sees the real reason.

Additive: signature unchanged; only the error detail improves.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@wytzepiet wytzepiet marked this pull request as draft May 31, 2026 18:30
@wytzepiet wytzepiet changed the title feat(runtime): surface the real JS exception from a failed async job feat(runtime): show the actual error when an async job throws May 31, 2026
@wytzepiet wytzepiet marked this pull request as ready for review May 31, 2026 20:53
@iota9star
Copy link
Copy Markdown
Member

Review findings:

  • The implementation improves the AsyncJobException path, but the PR description/test coverage should be tightened. A normal microtask throw such as Promise.resolve().then(() => { throw new Error(...) }) rejects the promise and does not make execute_pending_job() return Err, so this change does not surface every async callback throw as an immediate execute_pending_job() error.

    I verified this with a temporary test: the first execute_pending_job() returned Ok(true), then the queue drained without this error branch being hit.

    Suggested update: describe the behavior as improving diagnostics when QuickJS reports AsyncJobException from execute_pending_job(), and add a regression test that actually triggers that branch.

  • Merge note with fix: prevent JS stack overflow from crashing the process #15: libfjs/src/api/runtime.rs has a manual conflict around execute_pending_job(). The final merged version needs to preserve both behaviors: fix: prevent JS stack overflow from crashing the process #15 stack-executor wrapping and this PR detailed exception extraction via ctx.catch().

Verification run on this PR: cargo test --manifest-path libfjs/Cargo.toml passed locally with 533 tests.

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.

2 participants