Skip to content

Respect zero Node socket open timeouts - #7243

Merged
tim-smart merged 1 commit into
Effect-TS:mainfrom
AnnaSuSu:agent/fix-node-socket-zero-open-timeout
Aug 14, 2026
Merged

Respect zero Node socket open timeouts#7243
tim-smart merged 1 commit into
Effect-TS:mainfrom
AnnaSuSu:agent/fix-node-socket-zero-open-timeout

Conversation

@AnnaSuSu

Copy link
Copy Markdown
Contributor

What

NodeSocket.fromDuplex ignored openTimeout: 0, so an acquisition that never completed remained suspended instead of immediately failing with a socket open timeout.

Fix

Treat the timeout as configured whenever it is not undefined. This preserves zero-duration inputs and matches the handling used by the sibling Deno socket implementation.

Tests

  • Added a regression test proving a zero open timeout produces a SocketOpenError with the Timeout kind.
  • pnpm lint-fix
  • pnpm test --run packages/platform/node/test/NodeSocket.test.ts
  • pnpm check

@changeset-bot

changeset-bot Bot commented Aug 14, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b6d2619

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@effect/platform-node-shared Patch
effect Patch
@effect/ai-anthropic Patch
@effect/ai-openai Patch
@effect/ai-openai-compat Patch
@effect/ai-openrouter Patch
@effect/atom-react Patch
@effect/atom-solid Patch
@effect/atom-vue Patch
@effect/docgen Patch
@effect/doctest Patch
@effect/openapi-generator Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-deno Patch
@effect/platform-node Patch
@effect/sql-clickhouse Patch
@effect/sql-d1 Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-pglite Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-do Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/vitest Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

Copy link
Copy Markdown
Contributor

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name Current Size Previous Size Difference
basic.ts 6.92 KB 6.92 KB 0.00 KB (0.00%)
batching.ts 9.73 KB 9.73 KB 0.00 KB (0.00%)
brand.ts 6.50 KB 6.50 KB 0.00 KB (0.00%)
cache.ts 10.63 KB 10.63 KB 0.00 KB (0.00%)
config.ts 21.06 KB 21.06 KB 0.00 KB (0.00%)
differ.ts 20.00 KB 20.00 KB 0.00 KB (0.00%)
http-client.ts 21.55 KB 21.55 KB 0.00 KB (0.00%)
logger.ts 10.89 KB 10.89 KB 0.00 KB (0.00%)
metric.ts 8.86 KB 8.86 KB 0.00 KB (0.00%)
optic.ts 6.67 KB 6.67 KB 0.00 KB (0.00%)
pubsub.ts 14.91 KB 14.91 KB 0.00 KB (0.00%)
queue.ts 11.58 KB 11.58 KB 0.00 KB (0.00%)
schedule.ts 10.73 KB 10.73 KB 0.00 KB (0.00%)
schema-class.ts 19.61 KB 19.61 KB 0.00 KB (0.00%)
schema-fromJsonSchemaDocument.ts 29.57 KB 29.57 KB 0.00 KB (0.00%)
schema-representation-roundtrip.ts 25.80 KB 25.80 KB 0.00 KB (0.00%)
schema-string-transformation.ts 13.50 KB 13.50 KB 0.00 KB (0.00%)
schema-string.ts 10.99 KB 10.99 KB 0.00 KB (0.00%)
schema-template-literal.ts 15.30 KB 15.30 KB 0.00 KB (0.00%)
schema-toArbitrary.ts 21.74 KB 21.74 KB 0.00 KB (0.00%)
schema-toCodeDocument.ts 24.20 KB 24.20 KB 0.00 KB (0.00%)
schema-toCodecJson.ts 18.96 KB 18.96 KB 0.00 KB (0.00%)
schema-toEquivalence.ts 18.78 KB 18.78 KB 0.00 KB (0.00%)
schema-toFormatter.ts 18.65 KB 18.65 KB 0.00 KB (0.00%)
schema-toJsonSchemaDocument.ts 22.81 KB 22.81 KB 0.00 KB (0.00%)
schema-toRepresentation.ts 19.30 KB 19.30 KB 0.00 KB (0.00%)
schema.ts 18.86 KB 18.86 KB 0.00 KB (0.00%)
stm.ts 12.67 KB 12.67 KB 0.00 KB (0.00%)
stream.ts 9.67 KB 9.67 KB 0.00 KB (0.00%)

@AnnaSuSu
AnnaSuSu marked this pull request as ready for review August 14, 2026 01:50
Copilot AI lite review requested due to automatic review settings August 14, 2026 01:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes Node NodeSocket.fromDuplex open-timeout handling so that openTimeout: 0 is treated as an explicitly configured timeout (rather than being ignored due to falsy-check logic), aligning behavior with the Deno socket implementation and preventing indefinitely-suspended acquisitions.

Changes:

  • Treat openTimeout as configured whenever it is not undefined (so 0 is respected).
  • Add a regression test asserting openTimeout: 0 fails with SocketOpenError of kind Timeout.
  • Add a changeset for a patch release of @effect/platform-node-shared.

Reviewed changes

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

File Description
packages/platform/node-shared/src/NodeSocket.ts Fixes the open-timeout guard to respect zero-duration values by checking !== undefined.
packages/platform/node/test/NodeSocket.test.ts Adds a regression test ensuring openTimeout: 0 triggers a SocketOpenError(Timeout) rather than hanging.
.changeset/zero-node-socket-timeout.md Declares a patch changeset documenting the behavior fix.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@tim-smart
tim-smart merged commit 3bf8d52 into Effect-TS:main Aug 14, 2026
13 of 15 checks passed
@AnnaSuSu
AnnaSuSu deleted the agent/fix-node-socket-zero-open-timeout branch August 14, 2026 10:17
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