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, 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/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/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); + }); }); 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"] }