Skip to content

fix(typescript): handle tsserver crash instead of silently exiting#10238

Merged
davidfirst merged 5 commits intomasterfrom
fix/tsserver-silent-exit-on-crash
Mar 18, 2026
Merged

fix(typescript): handle tsserver crash instead of silently exiting#10238
davidfirst merged 5 commits intomasterfrom
fix/tsserver-silent-exit-on-crash

Conversation

@davidfirst
Copy link
Member

When tsserver child process crashes (e.g. SIGABRT from OOM on large workspaces), pending request promises in ProcessBasedTsServer were never resolved or rejected. Since the child process was the only thing keeping the Node.js event loop alive, the process would exit with code 0 silently — no error message, no completion of remaining validation steps.

process-based-tsserver.ts: Add tsServerProcess.on('exit') handler that rejects all pending request deferreds when the child process exits unexpectedly. This benefits all tsserver consumers (check-types, validate, watch, etc.).

validator.main.runtime.ts: Wrap getDiagnostic() in try-catch so a tsserver crash returns a proper ValidationResult with code 1 instead of crashing. Add batching (BATCH_SIZE=50) matching check-types.cmd.ts.

When tsserver child process crashes (e.g. SIGABRT from OOM), pending
request promises were never resolved/rejected causing Node.js to exit
with code 0 silently. Add exit handler on the child process to reject
all pending deferreds, and make validate's checkTypes resilient with
try-catch and batching.
Copilot AI review requested due to automatic review settings March 18, 2026 15:58
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves resilience around TypeScript server (tsserver) failures during type-checking by adding process-exit handling in the process-based tsserver wrapper and adding batching + error handling in the validator’s type-check step.

Changes:

  • Add a tsServerProcess.on('exit') handler and a helper to reject pending requests when tsserver exits.
  • Batch large getDiagnostic() runs in the validator and return a structured failure result on errors.
  • Ensure tsserver is killed on failure paths during validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
scopes/typescript/ts-server/process-based-tsserver.ts Adds child-process exit handling and a mechanism to reject outstanding requests when tsserver dies.
scopes/defender/validator/validator.main.runtime.ts Adds batching and error handling around tsserver.getDiagnostic() for validator type checks.

You can also share your feedback on Copilot code review. Take the survey.

- Add killedIntentionally flag to skip exit handler on normal shutdown
- Clean up tsServerProcess/readlineInterface on unexpected exit
- Use Object.values() for iterating deferreds
- Use defensive error formatting in catch block
Copilot AI review requested due to automatic review settings March 18, 2026 17:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves resilience of TypeScript tsserver-based type checking by handling unexpected tsserver termination and by batching diagnostic requests to avoid overwhelming tsserver during validation.

Changes:

  • Add an exit handler in ProcessBasedTsServer to log unexpected termination and reject pending requests.
  • Update validator type-check step to run diagnostics in batches and return a structured failure result on error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scopes/typescript/ts-server/process-based-tsserver.ts Adds unexpected-exit handling and a helper to reject pending in-flight requests.
scopes/defender/validator/validator.main.runtime.ts Wraps getDiagnostic() in batching + try/catch, and ensures tsserver is killed on failure.

You can also share your feedback on Copilot code review. Take the survey.

Copilot AI review requested due to automatic review settings March 18, 2026 18:34
@davidfirst davidfirst enabled auto-merge (squash) March 18, 2026 18:35
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Improves resilience of the TypeScript tsserver integration by handling unexpected tsserver process exits and reducing load when validating large file sets.

Changes:

  • Add unexpected child-process exit handling to ProcessBasedTsServer, including rejecting pending requests.
  • Track intentional vs. unexpected kills to avoid treating normal shutdown as an error.
  • Batch getDiagnostic() calls in the validator to reduce tsserver pressure, and return a structured error on failure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scopes/typescript/ts-server/process-based-tsserver.ts Detect unexpected tsserver exits and reject outstanding requests; track intentional kills.
scopes/defender/validator/validator.main.runtime.ts Batch diagnostics and handle tsserver failures by killing the server and returning an error result.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines 290 to 296
kill() {
this.killedIntentionally = true;
this.tsServerProcess?.kill();
this.tsServerProcess?.stdin?.destroy();
this.readlineInterface?.close();
this.tsServerProcess = null;
this.readlineInterface = null;
Comment on lines +88 to +90
} catch (err: any) {
tsserver.killTsServer();
const errMsg = err instanceof Error ? err.message : String(err);
@davidfirst davidfirst merged commit c9c2c76 into master Mar 18, 2026
16 checks passed
@davidfirst davidfirst deleted the fix/tsserver-silent-exit-on-crash branch March 18, 2026 19:24
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.

3 participants