[pull] main from TryGhost:main

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]);

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,

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) {

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;

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": {

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"]
+}

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, '<html></html>'),
@@ -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, '<html></html>');
+    });
+
+    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, '<html></html>');
+  });
+
+  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);
+  });
 });

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"]
 }