Skip to content

chore: increase timeout for connection requests to 30 seconds#2684

Merged
serhalp merged 2 commits intonpmx-dev:mainfrom
JounQin:patch-1
May 9, 2026
Merged

chore: increase timeout for connection requests to 30 seconds#2684
serhalp merged 2 commits intonpmx-dev:mainfrom
JounQin:patch-1

Conversation

@JounQin
Copy link
Copy Markdown
Contributor

@JounQin JounQin commented May 7, 2026

🔗 Linked issue

close #2683

🧭 Context

5s timeout for connection is tool short

📚 Description

Copilot AI review requested due to automatic review settings May 7, 2026 11:21
@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 7, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment May 9, 2026 1:02am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview May 9, 2026 1:02am
npmx-lunaria Ignored Ignored May 9, 2026 1:02am

Request Review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 7, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b45f783d-42b1-4591-a22b-22d20fc3515f

📥 Commits

Reviewing files that changed from the base of the PR and between ed65f10 and 5fb834c.

📒 Files selected for processing (1)
  • app/composables/useConnector.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/composables/useConnector.ts

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Adjusted connection and state refresh timeout values for improved robustness.

Walkthrough

The pull request introduces a shared CONNECT_TIMEOUT_MS (30,000ms) and updates the /connect POST and /state refresh $fetch calls in useConnector.ts to use this constant instead of the previous 5,000ms timeout.

Changes

Connector Timeout

Layer / File(s) Summary
Timeout constant
app/composables/useConnector.ts
Adds CONNECT_TIMEOUT_MS = 30000 constant for connector-related fetch calls.
Connect POST request
app/composables/useConnector.ts
Replaces the /connect POST call timeout from 5000ms to CONNECT_TIMEOUT_MS (30000ms).
State refresh request
app/composables/useConnector.ts
Replaces the /state refresh call timeout from 5000ms to CONNECT_TIMEOUT_MS (30000ms).
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title directly describes the main change: increasing timeout for connection requests to 30 seconds, which matches the core modification in the changeset.
Description check ✅ Passed The description references the linked issue (#2683) and provides context that the 5-second timeout was too short, relating to the changeset's purpose of increasing timeouts.
Linked Issues check ✅ Passed The pull request successfully addresses issue #2683 by introducing a CONNECT_TIMEOUT_MS constant set to 30 seconds and applying it to the /connect POST request and /state refresh request, directly resolving the timeout issue.
Out of Scope Changes check ✅ Passed All changes are within scope: introducing a timeout constant and applying it to connection-related requests, with no unrelated modifications to the codebase.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 7, 2026

Hello! Thank you for opening your first PR to npmx, @JounQin! 🚀

Here’s what will happen next:

  1. Our GitHub bots will run to check your changes.
    If they spot any issues you will see some error messages on this PR.
    Don’t hesitate to ask any questions if you’re not sure what these mean!

  2. In a few minutes, you’ll be able to see a preview of your changes on Vercel

  3. One or more of our maintainers will take a look and may ask you to make changes.
    We try to be responsive, but don’t worry if this takes a few days.

@JounQin JounQin changed the title Increase timeout for connection requests to 30 seconds chore: increase timeout for connection requests to 30 seconds May 7, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/composables/useConnector.ts`:
- Line 118: The post-connect state refresh uses a shorter timeout than the
updated connect timeout causing a slow connector to succeed then immediately be
marked disconnected; update refreshState() to use the same 30000ms timeout (or a
shared constant) as the /connect request (the timeout value set where connect is
called) and apply the same error-handling pattern used by connect (catch, log
with context, and preserve expected state) so both connect and refreshState use
a consistent timeout and error behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 01675d77-e96e-4190-844d-04ded3ecddc9

📥 Commits

Reviewing files that changed from the base of the PR and between 001d748 and ed65f10.

📒 Files selected for processing (1)
  • app/composables/useConnector.ts

Comment thread app/composables/useConnector.ts Outdated
Copy link
Copy Markdown
Contributor

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

Adjusts the connector client’s /connect request timeout to reduce false “timeout” failures during local connector startup/handshake, addressing issue #2683.

Changes:

  • Increased /connect request timeout from 5s to 30s in the connector composable.

Comment on lines 115 to 119
const response = await $fetch<ConnectResponse>(`http://127.0.0.1:${port}/connect`, {
method: 'POST',
body: { token },
timeout: 5000,
timeout: 30000,
})
@codecov
Copy link
Copy Markdown

codecov Bot commented May 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Member

@serhalp serhalp left a comment

Choose a reason for hiding this comment

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

👍🏼 I don't see why not

Comment thread app/composables/useConnector.ts Outdated
Comment thread app/composables/useConnector.ts Outdated
Comment thread app/composables/useConnector.ts
Co-authored-by: Philippe Serhal <philippe.serhal@gmail.com>
@serhalp serhalp enabled auto-merge May 9, 2026 01:00
@serhalp serhalp added this pull request to the merge queue May 9, 2026
Merged via the queue into npmx-dev:main with commit 49eb15f May 9, 2026
24 checks passed
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 9, 2026

Thanks for your first contribution, @JounQin! 🎉

We'd love to welcome you to the npmx community. Come and say hi on Discord! And once you've joined, visit npmx.wamellow.com to claim the contributor role.

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.

/connect timeout too short

3 participants