Tame miniapp runtime liveness warnings#3188
Conversation
📋 PR Review Helper📱 Mobile App Build✅ Ready to test! (commit 🕶️ ASG Client Build⏳ Waiting for build... 🔀 Test Locallygh pr checkout 3188 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0a3aaeef1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // waiting behind earlier queued work. Skipped during cold-start | ||
| // (timer still ticking from spawn). | ||
| if (record.readyAcked) { | ||
| armReadyNackTimer(record, STEADY_STATE_NACK_TIMEOUT_MS, cold = false) |
There was a problem hiding this comment.
Preserve NACK coverage for executor starvation
When the Android JS executor is already stuck before this submitted task starts—for example, a miniapp timer callback scheduled by scheduleTimer() enters a long/infinite CPU loop, and those timer callbacks do not have the soft watchdog—this line is never reached, so the 3s steady-state ready_nack is never armed for later host deliveries or pings. Previously dispatchToJs armed the timer before enqueueing, which still surfaced a dead context; now that case stays silent until the slower ping/foreground liveness path notices it. Please keep some starvation watchdog outside the executor or add equivalent watchdog coverage to queued timer work.
Useful? React with 👍 / 👎.
Summary
ready_nackframes as warnings instead of dev-overlay errorsTest
bun test mobile/modules/island/src/services/__tests__/MentraJSRouter.test.tsNote
Low Risk
Small timing/logging changes in miniapp delivery and observability; no auth, data, or crash-respawn behavior changes.
Overview
Reduces noisy “wedged runtime” signals when miniapp JS is merely backlogged on its per-context thread, and downgrades how those liveness NACKs appear in dev.
On Android
JSCRuntime.dispatchToJs, re-arming the steady-stateready_nackwatchdog moved inside the per-package executor task so the 3s timer measures time until__deliverruns, not time spent waiting behind earlier queued work. Cold-start NACK behavior is unchanged.In
MentraJSRouter,__errorframes with methodready_nacknow uselogger.warninstead oflogger.error, so they don’t surface like other miniapp failures in the dev overlay. Crash handling is unchanged (ready_nackstill isn’t treated as a crash). A unit test covers the warning path.Reviewed by Cursor Bugbot for commit d0a3aae. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Reduces false “wedged runtime” alerts by arming the steady-state NACK timer inside the per-context executor, and downgrades
ready_nackto a warning instead of a dev-overlay error.__delivertime, not queue delay.__error/ready_nackas a warning inMentraJSRouter; added a unit test to cover this behavior.Written for commit d0a3aae. Summary will update on new commits.