Skip to content

fix: MySQL QUERY_TIMEOUT kill + parseable NDJSON (#15) - #17

Merged
ChuTingzj merged 1 commit into
mainfrom
cursor/mysql-timeout-ndjson-26f5
Sep 21, 2026
Merged

ChuTingzj merged 1 commit into
mainfrom
cursor/mysql-timeout-ndjson-26f5

Conversation

@ChuTingzj

@ChuTingzj ChuTingzj commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Issues tracked

Issue Scope in this PR
Fixes #15 — MySQL SLEEP not killed + NDJSON/mux parse error Primary. Timeout kill path + parseable NDJSON after timeout/kill.
Related #16 — large MySQL result missing truncated:true Partial, same abort-hang root cause (not the same as SLEEP). Do not auto-close #16.

Fixes #15

#16 is not Fixes here. The abort-hang subset is included; UNION/CTE 400s and live large-result e2e remain follow-up.

#15 (primary) — timeout + parse

e2e against ffp-sql-sandbox@0.1.4 with SANDBOX_TIMEOUT_MS=10000:

  1. SELECT SLEEP(15) was not killed; the query returned success.
  2. Host sometimes saw Unexpected non-whitespace character after JSON at position 70 (mux/NDJSON corruption).

Root cause

  • MySQL SET SESSION MAX_EXECUTION_TIME is not a statement_timeout equivalent and does not interrupt SLEEP(). The runner had no wall-clock abort.
  • Docker attach demux concatenated stdout bytes with stderr bytes, so a kill/warning frame could splice into the middle of an NDJSON line.

Fix

  • Runner wall-clock watchdog at timeoutMs destroys the MySQL socket (SLEEP analog).
  • Demux stdout/stderr line-by-line per stream so stderr cannot splice into a stdout JSON event.
  • Map runner QUERY_TIMEOUT / Query execution was interrupted / Postgres statement timeout to code: 'TIMEOUT'.

Postgres statement_timeout does cancel pg_sleep. MySQL MAX_EXECUTION_TIME does not cancel SLEEP(). Dual timeout is watchdog at timeoutMs plus host container kill at timeoutMs + 2000ms.

#16 (related) — truncated:true hang

Included (same abort-hang cause as the timeout/kill path): after maxRows/maxBytes abort, the host used Promise.all([consumed, wait]), so a hung container.wait() / mysql2 end() delayed { truncated: true } until the client timed out. This PR returns truncated as soon as the consumer hits the cap and caps end() (~200ms) then destroy().

Follow-up (not this root cause, keep #16 open):

  • UNION / CTE attempts failing uncleanly (400 / MySQL recursion depth) rather than a clean truncated page.
  • Live Docker MySQL large-result e2e after runner image republish.

Tests

  • test/mysql-query-stream.test.ts — non-returning query is destroyed within timeoutMs; hung end() does not block close.
  • test/query-watchdog.test.ts — watchdog fires/clears; timeout message matching.
  • test/demux.test.ts / test/execute-sql.test.ts — stderr cannot splice a stdout NDJSON line; QUERY_TIMEOUT maps to TIMEOUT; maxRows abort returns truncated without waiting on wait().

Verified locally: pnpm test (75 pass), pnpm typecheck, pnpm build.

Follow-up (not in this PR)

  • Do not npm publish from this PR. After review, rebuild/publish the runner image and pin DEFAULT_SANDBOX_IMAGE.
  • Live Docker MySQL e2e (SLEEP(15) + large result) against the new runner digest.

Reviewer notes

Start with sandbox/mysql-query-stream.js (#15 watchdog), src/docker-types.ts (demuxStdout, #15 parse), and src/docker-executor.ts (timeout mapping + #16 return-on-truncate). Runner image consumers still on @0.1.4 digest will not pick up the watchdog until a runner republish.

Open in Web Open in Cursor 

MySQL MAX_EXECUTION_TIME does not interrupt SLEEP(); add a runner
wall-clock watchdog that destroys the client socket at timeoutMs.
Demux stdout/stderr as complete lines so kill-path stderr cannot splice
into NDJSON. Return truncated results without waiting on container.wait()
(#16). Map runner timeout errors to code TIMEOUT.

Co-authored-by: zhangjing <zhangjing798@qq.com>
@cursor cursor Bot changed the title fix: MySQL QUERY_TIMEOUT kill + parseable NDJSON after timeout fix: MySQL QUERY_TIMEOUT kill + parseable NDJSON (#15) Sep 21, 2026
@ChuTingzj
ChuTingzj marked this pull request as ready for review September 21, 2026 05:45

@ChuTingzj ChuTingzj left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict

Fixes #15 correctly: MySQL wall-clock watchdog (SLEEP), per-stream line demux (JSON splice), TIMEOUT mapping. #16 abort-hang subset fixed (return truncated without waiting on wait()); do not close #16 for UNION/CTE / live e2e follow-ups.

Findings

  1. Minor — truncate path: destroy() before finish() could theoretically reject if error is sync; prefer finish-first or ignore post-truncate errors.
  2. Ops — runner image / digest bump still required; @0.1.4 consumers will not get the watchdog until republish.

No must-fix. Do not merge from this bot.

});
try {
const query = connection.query({ sql, rowsAsArray: true });
await streamMysqlQuery(query, connection, limiter, writeEvent);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#15 root cause match. MAX_EXECUTION_TIME does not abort SLEEP(); wall-clock destroy() is the right MySQL lever. Tests lock destroy-within-timeoutMs and hung end() → destroy.

Residual: if connection.destroy() sync-emits error before finish(), the Promise can reject after a truncated end was already written. Prefer finish() before destroy(), or ignore errors once truncate is intended. Low likelihood with mysql2.

Comment thread src/docker-executor.ts
}
if (data.truncated || killedForLimit) {
return successResult(data);
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#15 parse fix looks right — complete lines per stream so stderr cannot splice mid-JSON. Both streams still feed one consumer; junk stderr lines are skipped by parseRunnerEvent. Good.

#16 partial: returning on data.truncated || killedForLimit without awaiting wait() is the abort-hang fix. Keep #16 open for UNION/CTE unclean failures + live large-result e2e after runner republish.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot did not reach a terminal state within the 8-minute wait, so this run is not approving. Human review is needed; no reviewers were assigned because the only GitHub contributor on these paths is the PR author. Tests and Cursor Security Agent already passed.

Open in Web View Automation 

Sent by Cursor Approval Agent: Pull Request Router and Approver

@ChuTingzj
ChuTingzj merged commit 9a3ed89 into main Sep 21, 2026
4 checks passed
ChuTingzj added a commit that referenced this pull request Sep 21, 2026
Pin DEFAULT_SANDBOX_IMAGE to the GHCR digest from Publish runner after #17 (Fixes #15 MySQL watchdog + demux). Bump npm package to 0.1.5.
ChuTingzj added a commit that referenced this pull request Sep 21, 2026
Pin DEFAULT_SANDBOX_IMAGE to the GHCR digest from Publish runner after #17 (MySQL watchdog + demux). Bump npm package to 0.1.5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants