Repository navigation
[api] Add timing collection and dump via environment variable - #64846
Andrew Branch (andrewbranch) merged 4 commits into
Conversation
Write versioned per-client timing snapshots to TS_API_TIMING_DIR during sync and async disposal, fetching final server timing before connection cleanup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9920adc3-3fc7-4d27-8ec1-c9c4490b9cf9
Record final timing query errors in the dump without writing to stdio, and cover dump activation and lifecycle without duplicating the timing schema. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9920adc3-3fc7-4d27-8ec1-c9c4490b9cf9
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Dispose disconnected JSON-RPC connections during final timing collection and preserve client-only measurements when server timing is disabled. Add focused regression coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9920adc3-3fc7-4d27-8ec1-c9c4490b9cf9
Ryan Cavanaugh (RyanCavanaugh)
left a comment
Ryan Cavanaugh (RyanCavanaugh)
left a comment
There was a problem hiding this comment.
(automated local review)
The environment-controlled timing dumps work in both API modes, and the async disconnect/disabled-server fixes check out. I confirmed one remaining shutdown regression at packages/typescript/src/api/sync/client.ts:201: after a synchronous filesystem callback throws, the final timing request can block close() indefinitely, preventing both the dump and cleanup.
The PR's existing "sync: preserves a dump after a filesystem callback throws" test reproduces the hang on Windows. A standalone repro finishes on base and with dumping disabled. Remembering the unrecoverable callback error in SyncRpcChannel and rejecting further I/O resolves it; local fix commit c8b1982436e85f6fd85e96b931d85c4843b5b367 is available but not pushed.
With that fix, all 966 API tests pass, including the existing callback-error test. Final npx hereby validate --api completed lint/format/build/API checks, but exited 1 because two unchanged fswatch tests require Windows symlink privileges unavailable here. Compiler baselines were unchanged.
|
The proposed change from the above comment is c8b1982 |
Remember callback failures before abandoning the response loop so final timing collection cannot block shutdown on a desynchronized channel. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9920adc3-3fc7-4d27-8ec1-c9c4490b9cf9
There was a problem hiding this comment.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Note
This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.
|
Sorry I clicked the Jake button when I meant to click the Copilot button 😄 The Jake response time is much faster though |
Adds a
TS_API_TIMING_DIRenvironment variable that causes the API client to collect timing info and write its results out to a file upon closing. I'm working on gathering an API benchmarking suite we can run on PRs with@typescript-bot api perf test thisor something.