Skip to content

Conversation

@jasisk
Copy link
Contributor

@jasisk jasisk commented Nov 17, 2025

Always set the socket response type to finished when finished is called. Prevents a race where finished sockets sometimes get logged as destroyed (since all sockets are eventually destroyed, even when they sent a response successfully).

Copilot AI review requested due to automatic review settings November 17, 2025 19:52
Copilot finished reviewing on behalf of jasisk November 17, 2025 19:53
Copy link

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 fixes a race condition in the request logger where the destroyed socket state could be incorrectly reported when a request finished successfully. The fix introduces a SUCCESS_SEMAPHORE symbol to explicitly track when a response has finished successfully, allowing the logger to distinguish between a completed response and a destroyed socket.

Key changes:

  • Added SUCCESS_SEMAPHORE symbol to track successful response completion
  • Modified response type detection logic to check for successful finish before checking for destroyed/aborted states
  • Updated quote style from single quotes to double quotes throughout the file
Comments suppressed due to low confidence (1)

src/telemetry/requestLogger.ts:74

  • The Response type definition in the finishLog function signature is missing the SUCCESS_SEMAPHORE property. Since res[SUCCESS_SEMAPHORE] is accessed at line 99, this property should be added to the type definition to maintain type safety. Update the type to: res: Response & { [LOGGED_SEMAPHORE]?: boolean; [SUCCESS_SEMAPHORE]?: boolean }
  res: Response & { [LOGGED_SEMAPHORE]?: boolean },

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jasisk jasisk force-pushed the js/fix/prevent-socket-destroyed-race branch from 42630bd to 62815b6 Compare November 17, 2025 20:21
@sonarqubecloud
Copy link

@djMax djMax merged commit 5c34ec4 into openapi-typescript-infra:main Nov 17, 2025
7 checks passed
@github-actions
Copy link

🎉 This PR is included in version 6.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jasisk jasisk deleted the js/fix/prevent-socket-destroyed-race branch November 18, 2025 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants