From 91a0275ca717b4126e05d764854c27e346fbeb0b Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Wed, 11 Nov 2026 13:39:43 -0500 Subject: [PATCH] fix(content): degrade missing-BLOB race to styled page, not raw 500 (#3103) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content page loads (/concepts/:slug, /tutorials/:slug) intermittently returned a raw 500 "Internal Server Error". Root cause: serveStoredSlug reads BLOB metadata and the BLOB in two separate queries (to avoid the HANA LOB-locator-expiry gotcha). When metadata hits but the separate BLOB read returns nothing — the inconsistent-read window during a concurrent publish/delta-write to the mutable ContentCurrent, or a transient HANA locator blip — blobRow.CONTENT / toBuffer(null) threw, and serveHandler's catch surfaced raw JSON instead of the styled page. - serveStoredSlug: guard null/missing/non-Buffer/zero-length BLOB → return 'not-found' (styled 404); emit content.blob.missing metric. Both HANA and SQLite branches now coerce via toBuffer + share the guard. - serveHandler catch: serveNotFound(res, slug, 500) (styled page, 500 kept for monitoring) instead of raw JSON — mirrors the catalog branch and the fail-open pages path. - serveNotFound: no-store on 5xx so a transient blip served as a styled 500 is never edge-cached for 60s (404s stay cacheable). - resolveContentBlob: same toBuffer(null) guard in both branches, so the error-page/nav resolver can't throw on the identical race. Covers srv and srv-qa (both shim over @tutorials/content). Test: publish a slug, null its content column (metadata intact), assert styled 404 not 500. Red before, green after. --- packages/content/content-store.js | 37 ++++++++++++++++++++++++++----- test/lib/content-store.test.js | 20 +++++++++++++++++ 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/packages/content/content-store.js b/packages/content/content-store.js index 508ef8f51..1f5ca0a98 100644 --- a/packages/content/content-store.js +++ b/packages/content/content-store.js @@ -323,7 +323,8 @@ export function createContentHandlers({ namespace = 'com.sap.developers.ims', ap } else { // slug-canonical: caller-canonicalizes const b = await SELECT.one.from(ContentCurrent).where({ slug }).columns('content'); - buffer = b ? await toBuffer(b.content) : null; + // #3103: toBuffer(null) throws — guard the raw column (same race as serveStoredSlug). + buffer = b?.content != null ? await toBuffer(b.content) : null; } return { contentHash: cur.contentHash, mimeType: cur.mimeType, version: cur.sourceVersion, buffer, source: 'current' }; } @@ -340,7 +341,8 @@ export function createContentHandlers({ namespace = 'com.sap.developers.ims', ap } else { // slug-canonical: caller-canonicalizes const b = await SELECT.one.from(ContentFiles).where({ slug, version: meta.version }).columns('content'); - buffer = b ? await toBuffer(b.content) : null; + // #3103: toBuffer(null) throws — guard the raw column (same race as serveStoredSlug). + buffer = b?.content != null ? await toBuffer(b.content) : null; } return { contentHash: meta.contentHash, mimeType: meta.mimeType, version: meta.version, buffer, source: 'files' }; } @@ -942,7 +944,9 @@ export function createContentHandlers({ namespace = 'com.sap.developers.ims', ap res.status(status); res.setHeader('Content-Type', `${resolved.mimeType}; charset=utf-8`); - res.setHeader('Cache-Control', 'public, max-age=60'); + // #3103: never let the edge cache a 5xx — a transient DB blip served as a + // styled 500 must not be pinned for 60s to later users. 404s stay cacheable. + res.setHeader('Cache-Control', status >= 500 ? 'no-store' : 'public, max-age=60'); res.setHeader('X-Content-Source', resolved.source === 'current' ? 'db-current' : 'db'); return res.send(decompressed); } catch (err) { @@ -1070,12 +1074,28 @@ export function createContentHandlers({ namespace = 'com.sap.developers.ims', ap `SELECT TOP 1 "CONTENT" FROM "${hanaTableName()}" WHERE "SLUG" = ? AND "VERSION" = ?`, [slug, meta.version] ); - contentBuf = blobRow.CONTENT; + // Raw SQL returns CONTENT as a Buffer (see comment above), but coerce + // defensively via toBuffer so a stream/string LOB isn't mis-flagged as a + // miss by the Buffer.isBuffer guard below — symmetric with the else branch. + contentBuf = blobRow?.CONTENT != null ? await toBuffer(blobRow.CONTENT) : undefined; } else { const blobRow = source === 'current' ? await SELECT.one.from(ContentCurrent).where({ slug }).columns('content') // slug-canonical: caller-canonicalizes : await SELECT.one.from(ContentFiles).where({ slug, version: meta.version }).columns('content'); // slug-canonical: caller-canonicalizes - contentBuf = await toBuffer(blobRow.content); + // Guard the raw column before toBuffer — toBuffer(null) itself throws, + // which would bubble past the Buffer.isBuffer check below. + contentBuf = blobRow?.content != null ? await toBuffer(blobRow.content) : undefined; + } + // Resilience (#3103): the metadata SELECT above hit, but this SEPARATE BLOB + // read came back empty — the inconsistent-read window when a concurrent + // publish/delta-write to ContentCurrent (overwrite-in-place, no version + // pin) lands between the two queries, or a transient HANA locator blip. + // Treat a missing/non-Buffer BLOB as a soft miss → caller serves the styled + // 404, rather than letting `gunzipSync(undefined)` throw into the raw-500 + // catch and show the user "Internal Server Error". + if (!Buffer.isBuffer(contentBuf) || contentBuf.length === 0) { + metrics.counter('content.blob.missing'); // observability for the race + return 'not-found'; } const decompressed = gunzipSync(contentBuf); const advisory = await computeAdvisory(slug); @@ -1339,7 +1359,12 @@ export function createContentHandlers({ namespace = 'com.sap.developers.ims', ap return serveNotFound(res, slug); } catch (err) { console.error('[content/serve]', err instanceof Error ? err.message : String(err)); - res.status(500).json({ error: 'Content retrieval failed' }); + metrics.counter('render.error'); // #3103: align with catalog-branch observability + // #3103: serve the styled error page (with 500 status for monitoring) + // instead of raw JSON "Content retrieval failed" — mirrors the catalog + // branch (line ~1260) and the fail-open pages path. A transient DB blip + // should never surface a bare "Internal Server Error" to the user. + return serveNotFound(res, slug, 500); } } diff --git a/test/lib/content-store.test.js b/test/lib/content-store.test.js index cb6925c26..1cfd73034 100644 --- a/test/lib/content-store.test.js +++ b/test/lib/content-store.test.js @@ -237,6 +237,26 @@ describe('content-store', () => { expect(res.headers['x-content-source']).toBe('cache'); }); + // Resilience: metadata row present but the BLOB column is empty/missing — + // the "metadata hit, BLOB read came back empty" race against a concurrent + // publish/delta-write to ContentCurrent. Must degrade to the styled 404, + // NOT bubble a TypeError into a raw 500 "Internal Server Error". See #3103. + it('returns the styled 404 (not a raw 500) when the BLOB is missing but metadata exists', async () => { + // Null out the content BLOB while leaving contentHash/version intact, + // reproducing the inconsistent-read window. + await UPDATE(ContentFiles).set({ content: null }).where({ slug: 'served-tut' }); + // Flush the LRU so the read hits the DB, not the warm cache. + const { invalidateContentCache } = await import('../../srv/lib/content-store.js'); + invalidateContentCache(); + + const res = await project.axios.get('/content/tutorials/served-tut', { + validateStatus: () => true + }); + + expect(res.status).not.toBe(500); + expect(res.status).toBe(404); + }); + it('redirects /tutorials/.html to canonical /tutorials/ with 301', async () => { const res = await project.axios.get('/content/tutorials/served-tut.html', { maxRedirects: 0,