Skip to content

Update https.Agent to leave keepAlive with the default value #6130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

karreiro
Copy link
Contributor

Re: #6062

User logs suggest a pattern where the first request succeeds (across different endpoints), but the second request fails with "Client network socket disconnected before secure TLS connection was established".

This points to a possible issue with socket reuse. When keepAlive is enabled, the first request's socket is saved for reuse, but by the time the second request tries to use it, the server may have already closed its side.

There are a few factors that can lead to this issue: aggressive server timeouts, dns rotation, or network setups that don’t guarantee the same backend each time.

This PR adds basic diagnostics and leave keepAlive with the default value (false) — forcing a new TCP connection for each request. If this hypnotises gets validated, this configuration must be defined at the RequestModeInput level.

@karreiro
Copy link
Contributor Author

/snapit

Copy link
Contributor

Coverage report

St.
Category Percentage Covered / Total
🟡 Statements
78.05% (+0.04% 🔼)
13000/16655
🟡 Branches
72.17% (-0% 🔻)
6335/8778
🟡 Functions
78.17% (+0.02% 🔼)
3370/4311
🟡 Lines
78.46% (+0.04% 🔼)
12310/15689
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / store-utils.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🔴
... / base-command.ts
36.36% (-3.64% 🔻)
0% 100%
36.36% (-3.64% 🔻)
🟢
... / store-export.ts
85% (-0.37% 🔻)
71.43% 88.89%
84.62% (-0.38% 🔻)
🔴
... / dev.ts
21.05% (-1.17% 🔻)
0% 0%
21.05% (-1.17% 🔻)

Test suite run success

3035 tests passing in 1316 suites.

Report generated by 🧪jest coverage report action from ed77b95

Copy link
Contributor

🫰✨ Thanks @karreiro! Your snapshot has been published to npm.

Test the snapshot by installing your package globally:

pnpm i -g @shopify/[email protected]

Tip

If you get an ETARGET error, install it with NPM and the flag --@shopify:registry=https://registry.npmjs.org

Caution

After installing, validate the version by running just shopify in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@karreiro karreiro closed this Jul 23, 2025
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.

1 participant