Skip to content

Conversation

DanieleAurilio
Copy link
Contributor

Related to #26819.
An optional timeout parameter has been added to the Deno.connect() interface. This parameter allows specifying a timeout (in milliseconds) within which the application must establish a connection. If the timeout is exceeded without successfully connecting, the operation is automatically aborted with an error. If the parameter is not provided, the default behavior remains unchanged (no timeout).
Currently, the timeout functionality is implemented only for TCP connections. Other connection types are not affected by this change.

Example usage:

try {
  const conn = await Deno.connect({
    hostname: "example.com",
    port: 5000,
    timeout: 5000, // 5 second timeout
  });

  console.log("Connection established successfully");  
  conn.close();
} catch (error) {
  if (error instanceof Deno.errors.TimedOut) {
    console.error("Connection timed out:", error.message);
  } else {
    console.error("Failed to connect:", error.message);
  }
}

If this solution is approved, the documentation will need to be updated to include details about the new timeout parameter.

@bartlomieju bartlomieju added this to the 2.2.0 milestone Nov 28, 2024
@DanieleAurilio
Copy link
Contributor Author

The commit 28ae134 add signal options to Deno.Connect() as discussed on issue #26819.

Found another issue #25265 related to timeout and signal on Deno.Connect.

@0f-0b
Copy link
Contributor

0f-0b commented Dec 2, 2024

I think the timeout option can be removed because signal is strictly more flexible. As an example, AbortSignal.any([req.signal, AbortSignal.timeout(30000)]) creates an AbortSignal that follows req.signal and also has a 30 second timeout.

@kt3k
Copy link
Member

kt3k commented Dec 2, 2024

I agree with @0f-0b. timeout option now doesn't seem useful to me

@DanieleAurilio
Copy link
Contributor Author

@kt3k @0f-0b Yes, after added the signal, also to me the timeout doesn't seem t o be useful, i'll remove it and try to understand why test failed.

@DanieleAurilio
Copy link
Contributor Author

@kt3k Timeout option removed from Deno.Connect() 👍

@kt3k kt3k changed the title feat(ext/ops) Add timeout parameter to Deno.Connect() feat(ext/ops) signal option to Deno.connect() Dec 6, 2024
@kt3k kt3k changed the title feat(ext/ops) signal option to Deno.connect() feat(ext/ops): add signal option to Deno.connect() Dec 6, 2024
@@ -1277,6 +1277,26 @@ Deno.test({
}
});

Deno.test(
{ permissions: { net: true } },
async function netTcpWithAbortSignal() {
Copy link
Member

Choose a reason for hiding this comment

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

Good test case 👍

kt3k added 2 commits December 10, 2024 18:15
Signed-off-by: Yoshiya Hinosawa <stibium121@gmail.com>
Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju Could you take another look?

@bartlomieju bartlomieju modified the milestones: 2.2.0, 2.3.0 Feb 17, 2025
@egfx-notifications
Copy link
Contributor

I can confirm for #25265 that it solves my use case.

Copy link
Member

@littledivy littledivy left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@littledivy littledivy merged commit f9a024a into denoland:main Apr 25, 2025
18 checks passed
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.

7 participants