From 2478784aae95558160920d7bec7288afec7691e6 Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Wed, 26 Aug 2026 00:17:02 -0400 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=8E=A8=20Improved=20resilience=20by?= =?UTF-8?q?=20skipping=20renders=20for=20disconnected=20clients=20(#30298)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ref INC-323 Under sustained load a Ghost site can queue requests for minutes. By the time a queued request reaches the renderer, the CDN or browser in front of it has usually timed out and destroyed the socket, but Ghost renders it anyway: a full template pass plus any {{#get}} queries the template makes, written to a socket nobody is reading. That waste is self-reinforcing. It occupies the single event loop, which lengthens the queue, which causes the next request to time out. In a recent incident an origin sat pegged at 100% CPU for two hours largely serving requests its CDN had already abandoned. The renderer now bails out when the response socket is already destroyed, checked both before the render starts and after it completes, since a slow render gives the client plenty of time to leave. The status is set to 499 (client closed request) so these are distinguishable in the access log from the phantom 200s they are currently recorded as. Only sockets Node has already destroyed are skipped, so there is no case where a client still waiting for a response gets it dropped. --- .../frontend/services/rendering/renderer.js | 15 ++++++ .../services/rendering/renderer.test.js | 52 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/ghost/core/core/frontend/services/rendering/renderer.js b/ghost/core/core/frontend/services/rendering/renderer.js index 93573f45ba0..aca5282cb03 100644 --- a/ghost/core/core/frontend/services/rendering/renderer.js +++ b/ghost/core/core/frontend/services/rendering/renderer.js @@ -15,6 +15,14 @@ const messages = { * @param {Object} data */ module.exports = function renderer(req, res, data) { + // CASE: client hung up while we fetched data. Rendering into a dead socket is + // wasted work that lengthens the queue under load. 499 = client closed request. + if (res.destroyed && !res.writableEnded) { + debug('Client gone before render, skipping: ' + req.originalUrl); + res.statusCode = 499; + return; + } + // Set response context setContext(req, res, data); @@ -44,6 +52,13 @@ module.exports = function renderer(req, res, data) { return req.next(err); } + // The render itself can take seconds; the client may have gone in the meantime. + if (res.destroyed && !res.writableEnded) { + debug('Client gone during render, discarding: ' + req.originalUrl); + res.statusCode = 499; + return; + } + // CASE: a {{#get}} or {{#collection}} helper aborted during rendering, so the // page contains fallback content — cap public caching at 60s so the broken page recovers quickly. if (res.locals?.degradedRender) { diff --git a/ghost/core/test/unit/frontend/services/rendering/renderer.test.js b/ghost/core/test/unit/frontend/services/rendering/renderer.test.js index ab0d4dd4fdb..312f1e5c208 100644 --- a/ghost/core/test/unit/frontend/services/rendering/renderer.test.js +++ b/ghost/core/test/unit/frontend/services/rendering/renderer.test.js @@ -1,3 +1,4 @@ +const assert = require('node:assert/strict'); const sinon = require('sinon'); const renderer = require('../../../../../core/frontend/services/rendering/renderer'); @@ -14,6 +15,9 @@ describe('Renderer', function () { res = { locals: {}, routerOptions: {}, + // an open response: both are always booleans on a real ServerResponse + destroyed: false, + writableEnded: false, // pre-set so templates.setTemplate returns early _template: 'index', render: sinon.stub().callsArgWith(2, null, ''), @@ -88,4 +92,52 @@ describe('Renderer', function () { sinon.assert.calledOnce(req.next); sinon.assert.notCalled(res.send); }); + + it('skips the render when the client hung up before it started', function () { + res.destroyed = true; + res.writableEnded = false; + + renderer(req, res, {}); + + sinon.assert.notCalled(res.render); + sinon.assert.notCalled(res.send); + assert.equal(res.statusCode, 499); + }); + + it('discards the html when the client hung up during the render', function () { + res.render = sinon.stub().callsFake(function (template, data, callback) { + res.destroyed = true; + res.writableEnded = false; + callback(null, ''); + }); + + renderer(req, res, {}); + + sinon.assert.calledOnce(res.render); + sinon.assert.notCalled(res.send); + assert.equal(res.statusCode, 499); + }); + + it('does not treat an already-sent response as a disconnect', function () { + res.destroyed = true; + res.writableEnded = true; + + renderer(req, res, {}); + + sinon.assert.calledOnceWithExactly(res.send, ''); + }); + + it('forwards render errors even when the client has hung up', function () { + req.next = sinon.spy(); + res.render = sinon.stub().callsFake(function (template, data, callback) { + res.destroyed = true; + callback(new Error('render failed')); + }); + + renderer(req, res, {}); + + // a broken template is worth logging whether or not anyone is still listening + sinon.assert.calledOnce(req.next); + sinon.assert.notCalled(res.send); + }); }); From ef758a324ce42ad8763567aa12b9946f3271581c Mon Sep 17 00:00:00 2001 From: Austin Burdine Date: Wed, 26 Aug 2026 00:39:27 -0400 Subject: [PATCH 2/3] Fixed missing import & add specific test tsconfig (#30299) no ref - using the same tsconfig for both tests and source allowed vitest globals to leak into source code and cause runtime errors - splitting tsconfig into an explicit test config ensures that missing imports correctly flag type errors --- ghost/core/core/server/ghost-server.ts | 1 + ghost/core/package.json | 2 +- ghost/core/test/tsconfig.json | 9 +++++++++ ghost/core/tsconfig.json | 5 ++--- 4 files changed, 13 insertions(+), 4 deletions(-) create mode 100644 ghost/core/test/tsconfig.json diff --git a/ghost/core/core/server/ghost-server.ts b/ghost/core/core/server/ghost-server.ts index b317f9e7ff0..2f26f5b0063 100644 --- a/ghost/core/core/server/ghost-server.ts +++ b/ghost/core/core/server/ghost-server.ts @@ -15,6 +15,7 @@ import type { Promisable } from 'type-fest'; import type * as express from 'express'; import type * as http from 'node:http'; import { promisify } from 'node:util'; +import assert from 'node:assert'; type ServerConfig = { host: string; diff --git a/ghost/core/package.json b/ghost/core/package.json index 06cc38ab278..0ad42da837b 100644 --- a/ghost/core/package.json +++ b/ghost/core/package.json @@ -72,7 +72,7 @@ "lint:frontend": "eslint 'core/frontend/**/*.js' --cache", "lint:test": "eslint 'test/**/*.js' --cache", "lint:code": "pnpm run '/^lint:(server|shared|frontend)$/'", - "lint:types": "eslint '**/*.ts' --cache && tsc --noEmit", + "lint:types": "eslint '**/*.ts' --cache && tsc --noEmit && tsc --noEmit -p test/tsconfig.json", "lint": "pnpm run '/^lint:(server|shared|frontend|test|types)$/'" }, "dependencies": { diff --git a/ghost/core/test/tsconfig.json b/ghost/core/test/tsconfig.json new file mode 100644 index 00000000000..a2d42eb6064 --- /dev/null +++ b/ghost/core/test/tsconfig.json @@ -0,0 +1,9 @@ +{ + /* Vitest's globals live here only, so they can't mask a missing import in core source. */ + "extends": "../tsconfig.json", + "compilerOptions": { + "noEmit": true, + "types": ["node", "vitest/globals"] + }, + "include": ["**/*.ts", "../bin/**/*.d.ts", "../core/**/*.d.ts", "../types/**/*.d.ts"] +} diff --git a/ghost/core/tsconfig.json b/ghost/core/tsconfig.json index ff821334c2b..9f3f83a1ff1 100644 --- a/ghost/core/tsconfig.json +++ b/ghost/core/tsconfig.json @@ -30,8 +30,7 @@ // "rootDirs": [], /* Allow multiple folders to be treated as one when resolving modules. */ // "typeRoots": [], /* Specify multiple folders that act like './node_modules/@types'. */ "types": [ - "node", - "vitest/globals" + "node" ] /* Specify type package names to be included without being referenced in a source file. */, // "allowUmdGlobalAccess": true, /* Allow accessing UMD globals from modules. */ // "moduleSuffixes": [], /* List of file name suffixes to search when resolving a module. */ @@ -102,5 +101,5 @@ // "skipDefaultLibCheck": true, /* Skip type checking .d.ts files that are included with TypeScript. */ "skipLibCheck": true /* Skip type checking all .d.ts files. */ }, - "include": ["bin/**/*.ts", "core/**/*.ts", "test/**/*.ts", "types/**/*.d.ts"] + "include": ["bin/**/*.ts", "core/**/*.ts", "types/**/*.d.ts"] } From 01864533ffba878154b3f611a1ead069c5f26618 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Wed, 26 Aug 2026 09:31:38 +0100 Subject: [PATCH 3/3] Fixed gift redemption recipient name (#30288) no issue Gift redemption should keep a personalized gift addressed to its intended recipient, even when the buyer or another member opens the link while signed in. - Preferred the stored recipient name over the signed-in member name - Retained the member name as a fallback for gifts without a recipient name - Added regression coverage for signed-in viewers --- .../pages/beta-gift-redemption-page.jsx | 4 ++-- .../pages/gift-redemption-page.test.jsx | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/apps/portal/src/components/pages/beta-gift-redemption-page.jsx b/apps/portal/src/components/pages/beta-gift-redemption-page.jsx index de0e127a470..59f36c59f36 100644 --- a/apps/portal/src/components/pages/beta-gift-redemption-page.jsx +++ b/apps/portal/src/components/pages/beta-gift-redemption-page.jsx @@ -59,7 +59,7 @@ const BetaGiftRedemptionPage = () => { const { action, brandColor, doAction, member, pageData, site } = useContext(AppContext); const gift = pageData?.gift; const isLoggedIn = !!member; - const [name, setName] = useState(member?.name || gift?.recipient_name || ''); + const [name, setName] = useState(gift?.recipient_name || member?.name || ''); const [email, setEmail] = useState(member?.email || ''); const [errors, setErrors] = useState({}); const [showDetails, setShowDetails] = useState(false); @@ -68,7 +68,7 @@ const BetaGiftRedemptionPage = () => { useEffect(() => { // Prefill with the recipient name the buyer entered, so the gift card // is personal before the recipient types anything. - setName(member?.name || gift?.recipient_name || ''); + setName(gift?.recipient_name || member?.name || ''); setEmail(member?.email || ''); setErrors({}); }, [member?.email, member?.name, gift?.recipient_name]); diff --git a/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx b/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx index eb9b495b623..96091f2bb9a 100644 --- a/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx +++ b/apps/portal/test/unit/components/pages/gift-redemption-page.test.jsx @@ -123,6 +123,24 @@ describe.each([ }); describe('BetaGiftRedemptionPage', () => { + test('shows the intended recipient name when a different member is logged in', () => { + const personalizedGift = { + ...gift, + buyer_name: 'Morgan', + recipient_name: 'Taylor', + }; + const { getByText, queryByText } = renderGiftRedemptionPage(BetaGiftRedemptionPage, { + member: member.free, + pageData: { + token: 'gift-token-123', + gift: personalizedGift, + }, + }); + + expect(getByText('Taylor')).toBeInTheDocument(); + expect(queryByText(member.free.name)).not.toBeInTheDocument(); + }); + test('presents the buyer details and prefills the intended recipient name', () => { const personalizedGift = { ...gift,