Skip to content

Added shared keep-alive agent to external HTTP requests#28228

Open
rmgpinto wants to merge 1 commit into
mainfrom
keep-alive-optimization
Open

Added shared keep-alive agent to external HTTP requests#28228
rmgpinto wants to merge 1 commit into
mainfrom
keep-alive-optimization

Conversation

@rmgpinto
Copy link
Copy Markdown
Contributor

@rmgpinto rmgpinto commented May 28, 2026

ref https://linear.app/ghost/issue/BER-3690

  • got.extend in request-external.js had no agent configured, so every oEmbed / webmention / recommendation / image probe opened a fresh socket
  • on a NAT-gatewayed VPC this holds the gateway at its connection-rate ceiling and causes port-collision drops that gateway sizing won't fix
  • adds a shared http/https Agent (keepAlive: true, keepAliveMsecs: 60s, maxSockets: 256) wired into the central got instance
  • routes probe-image-size through externalRequest.stream so the parallel cover/author/og/twitter/logo fetches from meta/image-dimensions.js reuse the same pool (and pick up the existing SSRF protections)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 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: af16f7b5-1a31-4ab7-895d-bf17927cd6a4

📥 Commits

Reviewing files that changed from the base of the PR and between 648ea51 and 2b6ec92.

📒 Files selected for processing (4)
  • ghost/core/core/server/lib/image/image-size.js
  • ghost/core/core/server/lib/image/image-utils.js
  • ghost/core/core/server/lib/request-external.js
  • ghost/core/test/unit/server/lib/image/image-utils.test.js
🚧 Files skipped from review as they are similar to previous changes (3)
  • ghost/core/core/server/lib/image/image-size.js
  • ghost/core/core/server/lib/request-external.js
  • ghost/core/core/server/lib/image/image-utils.js

Walkthrough

This PR improves image probing reliability and connection efficiency by introducing three coordinated changes: (1) adding shared keep-alive HTTP/HTTPS agents to the external request client so outbound connections are pooled and reused, (2) creating a local probe helper that uses the pooled client instead of probe-image-size's default client, and (3) refining the timeout mechanism in _probeImageSizeFromUrl to explicitly destroy streams on timeout expiry and clear timers via .finally() blocks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main objective of the PR: adding a shared keep-alive agent to external HTTP requests, which directly addresses the core change across all modified files.
Description check ✅ Passed The description is directly related to the changeset, providing clear context about the problem (socket exhaustion on NAT-gatewayed VPCs), the solution (shared keep-alive agents), and implementation details (SSRF protections and connection pooling).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch keep-alive-optimization

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.

Copy link
Copy Markdown

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

Introduces a shared keep-alive HTTP/HTTPS agent pool for outbound got requests in request-external.js to stop one-socket-per-request churn (which was saturating the NAT gateway's connection-rate limit). Image probing in probe-image-size is rewired through the same got instance so the parallel meta/image-dimensions fetches reuse the pool and pick up the existing SSRF protections; the outer "probe unresponsive" timeout now destroys the underlying stream so the pooled socket isn't leaked.

Changes:

  • Add shared http.Agent / https.Agent (keep-alive, keepAliveMsecs: 60000, maxSockets/maxFreeSockets: 256) and wire them into the central got instance via gotOpts.agent.
  • Replace direct use of probe-image-size in image-utils.js with a wrapper that streams the body through externalRequest.stream, exposing .stream on the returned promise.
  • Update _probeImageSizeFromUrl to destroy the probe stream when the outer timeout wins, and clear the timeout in a finally to avoid leaks.

Reviewed changes

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

File Description
ghost/core/core/server/lib/request-external.js Adds shared keep-alive agents and registers them in the got.extend options.
ghost/core/core/server/lib/image/image-utils.js Wraps probe-image-size over externalRequest.stream, exposing the underlying stream for cancellation.
ghost/core/core/server/lib/image/image-size.js Reworks the probe/timeout race to destroy the stream on timeout and clear the timer on settle.

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

Comment thread ghost/core/core/server/lib/request-external.js
Comment thread ghost/core/core/server/lib/image/image-utils.js
ref https://linear.app/ghost/issue/BER-3690

- got.extend in request-external.js had no agent configured, so every
oEmbed / webmention / recommendation / image probe opened a fresh socket
- on a NAT-gatewayed VPC this holds the gateway at its connection-rate
ceiling and causes port-collision drops that gateway sizing won't fix
- adds a shared http/https Agent (keepAlive: true, keepAliveMsecs: 60s,
maxSockets: 256) wired into the central got instance
- routes probe-image-size through externalRequest.stream so the parallel
cover/author/og/twitter/logo fetches from meta/image-dimensions.js reuse
the same pool (and pick up the existing SSRF protections)
@rmgpinto rmgpinto force-pushed the keep-alive-optimization branch 2 times, most recently from af82bc6 to 2b6ec92 Compare May 28, 2026 10:13
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

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.

2 participants