Skip to content

Conversation

Copy link

Copilot AI commented Dec 8, 2025

Investigation and Fix for PR #1100

  • Analyzed PR connection: Do not throw on socket data if stream parser is null #1100 - removes flaky test and reorganizes test setup
  • Identified root cause: "write after end" error in stream-management timeout
  • Reviewed stream-management code to understand timeout handling
  • Identified where errors occur when socket is detached
  • Implemented fix in connection/index.js to check socket exists before writing
  • Verified fix with standalone test
  • Applied PR connection: Do not throw on socket data if stream parser is null #1100 test changes (removed flaky test, reorganized setup)
  • Addressed code review feedback - improved error handling in afterAll
  • Security scan passed - no vulnerabilities found
  • All changes complete and ready for review

Summary

Successfully fixed the failing test in PR #1100 by addressing the underlying "write after end" error and applying the PR's test improvements.

Root Cause Analysis

The test failure occurred because:

  1. Test called _detachSocket() to simulate network disconnection
  2. This set socket to null but left timers running in stream-management
  3. When timers fired, they called entity.send() which tried to write to the null socket
  4. This caused a synchronous "write after end" error before the Promise could catch it

Changes Made

1. packages/connection/index.js

Added null check before writing to socket:

if (!this.socket) {
  throw new Error("Socket is not connected");
}

2. test/stream-management.test.js

  • Moved server.enableModules(["smacks"]) to beforeAll hook
  • Added beforeEach hook for server restart
  • Changed cleanup to afterAll with try/finally for robust error handling
  • Removed flaky "pings do not prevent timeout" test
  • Removed duplicate server setup from individual tests

Security Summary

✅ CodeQL scan passed with no alerts
✅ No vulnerabilities introduced by changes

The fix makes the error handling more robust and prevents confusing "write after end" errors when the socket is disconnected.

Original prompt

Hey, can you help me investigate the failing test at #1100


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI requested a review from sonnyp December 8, 2025 17:49
@sonnyp sonnyp closed this Dec 8, 2025
@sonnyp sonnyp deleted the copilot/investigate-failing-test-1100 branch December 8, 2025 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants