[PR #11717] fix: memory leaks, error handling, and retry logic improvements #13905

Closed
opened 2026-02-16 18:18:44 -05:00 by yindo · 0 comments
Owner

Original Pull Request: https://github.com/anomalyco/opencode/pull/11717

State: closed
Merged: No


Summary

This PR addresses several memory leaks and error handling issues discovered during a deep code review:

Bug Fixes

HIGH Priority:

  1. Timer Memory Leak in withTimeout (src/util/timeout.ts)

    • The timeout timer was only cleared on promise resolution, not rejection
    • Changed .then() to .finally() to ensure cleanup in all code paths
  2. Event Listener Memory Leak in OAuth Flow (src/mcp/index.ts)

    • Event listeners (error, exit) were not removed after one fired
    • Changed .on() to .once() for subprocess handlers

MEDIUM Priority:

  1. Silent Cache Clearing Failure (src/global/index.ts)
    • Version file was written even when cache deletion failed, corrupting cache state
    • Moved version write inside try block and added error logging

LOW Priority:

  1. Inconsistent Retry Logic Return Value (src/session/retry.ts)
    • retryable() returned raw JSON.stringify() for unrecognized errors
    • Now returns undefined for non-retryable errors (consistent behavior)
    • Updated test to match new correct behavior

Files Changed

  • packages/opencode/src/util/timeout.ts - Timer cleanup fix
  • packages/opencode/src/mcp/index.ts - Event listener fix
  • packages/opencode/src/global/index.ts - Cache error handling
  • packages/opencode/src/session/retry.ts - Retry logic fix
  • packages/opencode/test/session/retry.test.ts - Updated test

Testing

  • All 841 tests pass
  • TypeScript typecheck passes for all 12 packages

Checklist

  • Code follows project style guidelines
  • All tests pass
  • No breaking changes
  • Memory leak issues addressed
**Original Pull Request:** https://github.com/anomalyco/opencode/pull/11717 **State:** closed **Merged:** No --- ## Summary This PR addresses several memory leaks and error handling issues discovered during a deep code review: ### Bug Fixes **HIGH Priority:** 1. **Timer Memory Leak in `withTimeout`** (`src/util/timeout.ts`) - The timeout timer was only cleared on promise resolution, not rejection - Changed `.then()` to `.finally()` to ensure cleanup in all code paths 2. **Event Listener Memory Leak in OAuth Flow** (`src/mcp/index.ts`) - Event listeners (`error`, `exit`) were not removed after one fired - Changed `.on()` to `.once()` for subprocess handlers **MEDIUM Priority:** 3. **Silent Cache Clearing Failure** (`src/global/index.ts`) - Version file was written even when cache deletion failed, corrupting cache state - Moved version write inside try block and added error logging **LOW Priority:** 4. **Inconsistent Retry Logic Return Value** (`src/session/retry.ts`) - `retryable()` returned raw `JSON.stringify()` for unrecognized errors - Now returns `undefined` for non-retryable errors (consistent behavior) - Updated test to match new correct behavior ## Files Changed - `packages/opencode/src/util/timeout.ts` - Timer cleanup fix - `packages/opencode/src/mcp/index.ts` - Event listener fix - `packages/opencode/src/global/index.ts` - Cache error handling - `packages/opencode/src/session/retry.ts` - Retry logic fix - `packages/opencode/test/session/retry.test.ts` - Updated test ## Testing - All 841 tests pass - TypeScript typecheck passes for all 12 packages ## Checklist - [x] Code follows project style guidelines - [x] All tests pass - [x] No breaking changes - [x] Memory leak issues addressed
yindo added the pull-request label 2026-02-16 18:18:44 -05:00
yindo closed this issue 2026-02-16 18:18:44 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: anomalyco/opencode#13905