From 67a202cdfd729b5904644d1a33e4c1c1580c5268 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 20 Oct 2026 18:57:21 +0000 Subject: [PATCH 1/2] content-node: let go of unread answers; discover loads only an event From the review of everything merged since #485 (spec 006). - An answer whose body is never read holds its connection until it is collected. Solr's refusals (a query it cannot parse; anyone can send one) and every report-service answer were never read; each is now let go at once. - /data/discover/{id} asked for every relationship of the object, both ways, before checking it was an event: for a species or a database, hundreds of thousands, only to answer a 422. It now asks the object's class and whether it has any relationship first, answers the 404 or the 422 from that as before, and loads only an event. - /search/icon/{identifier}: a count with no document (which Solr does not send at its default rows) answered nothing at all, the response never ended; it is now the Solr failure its comment promised. - warm.ts, finding the edge reporting the wrong release, says a purge may be what is needed, not only a restart. Each new spec shown failing without its fix. Co-Authored-By: Claude Opus 5.5 --- tools/content-node/discover.spec.mjs | 20 +++++++ tools/content-node/icon.spec.mjs | 9 ++++ tools/content-node/report.ts | 4 +- tools/content-node/service.ts | 17 +++++- tools/content-node/solr.ts | 6 ++- tools/content-node/unread-answers.spec.ts | 66 +++++++++++++++++++++++ tools/content-node/warm.spec.ts | 2 + tools/content-node/warm.ts | 3 +- 8 files changed, 122 insertions(+), 5 deletions(-) create mode 100644 tools/content-node/unread-answers.spec.ts diff --git a/tools/content-node/discover.spec.mjs b/tools/content-node/discover.spec.mjs index c403fa37..d7060667 100644 --- a/tools/content-node/discover.spec.mjs +++ b/tools/content-node/discover.spec.mjs @@ -205,6 +205,9 @@ const LOADED = { const graph = ({ load = [LOADED] } = {}) => (cypher, params) => { + // The object's class, and whether it has any relationship, asked first. + if (/AS related/.test(cypher)) + return [{ labels: (load[0] ?? LOADED).root.labels, related: load.length > 0 }]; if (/MATCH \(n\)-\[r\]-\(m\)/.test(cypher)) return load; if (/DBInfo/.test(cypher)) return [{ version: 97 }]; // The edit's author, with no stoichiometry on the relationship: once. @@ -258,6 +261,23 @@ describe("/data/discover/{identifier}: Java's errors", () => { expect([response.status, (await response.json()).messages]).toEqual([422, [null]]); }); + it('never loads what is not found, or not an event: only an event is worth its relationships', async () => { + const loads = () => asked.filter((cypher) => /MATCH \(n\)-\[r\]-\(m\)/.test(cypher)); + for (const [load, status] of [ + [[], 404], + [[{ ...LOADED, root: { ...ROOT, labels: ['DatabaseObject', 'Species'] } }], 422], + ]) { + answer = graph({ load }); + asked.length = 0; + expect((await fetch(`${base}/ContentService/data/discover/48887`)).status).toBe(status); + expect(loads()).toEqual([]); + } + answer = graph(); + asked.length = 0; + expect((await fetch(`${base}/ContentService/data/discover/R-HSA-5`)).status).toBe(200); + expect(loads()).toHaveLength(1); + }); + it("loads an event with graph-core's findByStId: every relationship, both ways, unsorted", async () => { answer = graph(); asked.length = 0; diff --git a/tools/content-node/icon.spec.mjs b/tools/content-node/icon.spec.mjs index 5bced6d5..b0da67ee 100644 --- a/tools/content-node/icon.spec.mjs +++ b/tools/content-node/icon.spec.mjs @@ -253,6 +253,15 @@ describe('the icon endpoints, against a stand-in Solr', () => { ]); }); + it('/icon/{id} found but sent no document is a failure, never an empty 200 to cache', async () => { + answer = found([], 1); + const response = await fetch(`${base}/ContentService/search/icon/R-ICO-013428`); + expect([response.status, (await response.json()).messages]).toEqual([ + 500, + ['Solr exception occurred'], + ]); + }); + it("/icon/{id} found nowhere is Java's 404, naming the id as given", async () => { answer = found([]); const response = await fetch(`${base}/ContentService/search/icon/R-ICO-013428.json`); diff --git a/tools/content-node/report.ts b/tools/content-node/report.ts index 81a5516e..3bacd725 100644 --- a/tools/content-node/report.ts +++ b/tools/content-node/report.ts @@ -112,7 +112,9 @@ function send(mapping: string, info: ReportInfo, body: ReportTerm | ReportTerm[] ), signal: AbortSignal.timeout(REPORT_TIMEOUT_MS), }) - .then((response) => { + .then(async (response) => { + // Nothing in the answer is read; unread, it would hold the connection. + await response.body?.cancel(); if (response.status !== 200) console.error(`[REP001] The report service answered ${response.status}; no report made`); }) diff --git a/tools/content-node/service.ts b/tools/content-node/service.ts index a5ad655c..7a1a8a4b 100644 --- a/tools/content-node/service.ts +++ b/tools/content-node/service.ts @@ -1625,6 +1625,19 @@ export const endpoints: Endpoint[] = [ const missing = () => notFound(request, `${raw} not found`); const { where, params } = resolveId(raw); if (where === 'false') return missing(); + // What Java decides from the load below -- found, an event -- asked + // first, alone. The load gathers every relationship of the object, and + // for a species or a database that is hundreds of thousands, to answer + // a 422; only an event is loaded. + const [kind] = await read<{ labels: string[]; related: boolean }>( + `MATCH (n:DatabaseObject) WHERE ${where} + WITH n LIMIT 1 + RETURN labels(n) AS labels, EXISTS((n)--()) AS related`, + params + ); + if (!kind?.related) return missing(); + if (!isA(schemaClassOf(kind.labels), 'Event')) + return javaError(request, 422, 'UNPROCESSABLE_ENTITY', null); // graph-core's findByStId / findByDbId, as written but for the id's // test: every relationship, both ways, in the order the store gives. const [row] = await read( @@ -3485,8 +3498,8 @@ export const endpoints: Endpoint[] = [ type: 'application/json', // numFound > 0 with no documents cannot happen at Solr's default rows. // If it did, this throws and fromSolr answers its Solr failure -- not - // Java's NullPointerException answer, as it did before it was typed. - body: present(searchResult(answer, false).entries)[0], + // Java's NullPointerException answer, and never a 200 with no body. + body: present(present(searchResult(answer, false).entries)[0]), }; }); }, diff --git a/tools/content-node/solr.ts b/tools/content-node/solr.ts index 93eebcd8..4b2a385b 100644 --- a/tools/content-node/solr.ts +++ b/tools/content-node/solr.ts @@ -188,8 +188,12 @@ export async function solr( ...(method === 'GET' ? {} : { body: form }), signal: until, }); - if (!response.ok) + if (!response.ok) { + // Its body unread holds the connection until collected: one request + // that Solr refuses (a query it cannot parse) is enough to ask it. + await response.body?.cancel(); throw Object.assign(new Error(`Solr answered ${response.status}`), { solr: true }); + } // What the handler sent (JSON's null too); the readers check each part they use. return (await response.json()) as SolrAnswer | null; } catch (failure) { diff --git a/tools/content-node/unread-answers.spec.ts b/tools/content-node/unread-answers.spec.ts new file mode 100644 index 00000000..a133f0a1 --- /dev/null +++ b/tools/content-node/unread-answers.spec.ts @@ -0,0 +1,66 @@ +// @vitest-environment node +// +// An answer whose body is never read holds its connection until it is +// collected. Solr's refusals (a query it cannot parse) and the report +// service's answers are never read, and either can be asked for by anyone, +// so each is let go at once. +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { sendReport } from './report.ts'; +import { solr } from './solr.ts'; + +/** An answer whose body says whether it was let go. */ +function answer(status: number) { + const body = { cancelled: false }; + const stream = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"error":{"msg":"cannot parse"}}')); + }, + cancel() { + body.cancelled = true; + }, + }); + return { response: new Response(stream, { status }), body }; +} + +const saved = { ...process.env }; +afterEach(() => { + vi.unstubAllGlobals(); + process.env = { ...saved }; +}); + +describe('an answer nothing reads', () => { + it("is let go when Solr refuses the query, and the refusal is still Solr's", async () => { + process.env['SOLR_URL'] = 'http://127.0.0.1:1/solr/reactome'; + const { response, body } = answer(400); + vi.stubGlobal( + 'fetch', + vi.fn(async () => response) + ); + await expect(solr('/select', [['q', '(']])).rejects.toMatchObject({ + message: 'Solr answered 400', + solr: true, + }); + expect(body.cancelled).toBe(true); + }); + + it("is let go when it is the report service's, whatever its status", async () => { + process.env['REPORT_URL'] = 'http://127.0.0.1:1'; + for (const status of [200, 500]) { + const { response, body } = answer(status); + vi.stubGlobal( + 'fetch', + vi.fn(async () => response) + ); + vi.spyOn(console, 'error').mockImplementation(() => {}); + sendReport( + 'notfound', + { agent: null, ip: '203.0.113.1', release: '97' }, + { + term: 'XYZ', + resource: '', + } + ); + await vi.waitFor(() => expect(body.cancelled).toBe(true)); + } + }); +}); diff --git a/tools/content-node/warm.spec.ts b/tools/content-node/warm.spec.ts index 2fa8dc9c..e729debf 100644 --- a/tools/content-node/warm.spec.ts +++ b/tools/content-node/warm.spec.ts @@ -114,6 +114,8 @@ describe('main', () => { const [code, output] = await run(['https://site', '99'], { fetch }); expect(code).toBe(2); expect(output).toContain('reports release 98, not 99'); + // A restart cannot help an edge still holding the old number; a purge can. + expect(output).toContain('purge the edge again'); expect(asked).toEqual(['https://site/ContentService/data/database/version']); }); diff --git a/tools/content-node/warm.ts b/tools/content-node/warm.ts index 3870943d..9da363c2 100644 --- a/tools/content-node/warm.ts +++ b/tools/content-node/warm.ts @@ -143,7 +143,8 @@ export async function main( const serving = String(await getJson(`${CS}/data/database/version`)); if (serving !== release) { log( - `the content API reports release ${serving}, not ${release}: restart it on the new release first` + `the content API reports release ${serving}, not ${release}: restart it on the new release, ` + + 'or, if it already is, purge the edge again -- it may hold the old number' ); return 2; } From f9d89d7ed80046678eb7c4abd499e2fcd926ea33 Mon Sep 17 00:00:00 2001 From: Adam Wright Date: Tue, 20 Oct 2026 19:10:50 +0000 Subject: [PATCH 2/2] Review of the content-node fixes: the icon was already safe; cancels quiet - /search/icon/{identifier}: the change is undone. searchResult leaves `entries` out when there are no documents, so the line already threw and fromSolr answered Solr's failure; the commit before said otherwise. Its spec stays, to hold that answer, and the comment says why it holds. - A body cancel that fails (a body already broken) is ignored: it could replace the Solr error, and turned the report service's "answered 500" log line into "unavailable". The report's status is logged first; a spec shows it (failing on the commit before). Co-Authored-By: Claude Opus 5.5 --- tools/content-node/icon.spec.mjs | 2 +- tools/content-node/report.ts | 5 +++-- tools/content-node/service.ts | 7 +++--- tools/content-node/solr.ts | 2 +- tools/content-node/unread-answers.spec.ts | 26 +++++++++++++++++++++++ 5 files changed, 35 insertions(+), 7 deletions(-) diff --git a/tools/content-node/icon.spec.mjs b/tools/content-node/icon.spec.mjs index b0da67ee..f3dda775 100644 --- a/tools/content-node/icon.spec.mjs +++ b/tools/content-node/icon.spec.mjs @@ -253,7 +253,7 @@ describe('the icon endpoints, against a stand-in Solr', () => { ]); }); - it('/icon/{id} found but sent no document is a failure, never an empty 200 to cache', async () => { + it("/icon/{id} found but sent no document is Solr's failure, never an empty 200 to cache", async () => { answer = found([], 1); const response = await fetch(`${base}/ContentService/search/icon/R-ICO-013428`); expect([response.status, (await response.json()).messages]).toEqual([ diff --git a/tools/content-node/report.ts b/tools/content-node/report.ts index 3bacd725..25c26668 100644 --- a/tools/content-node/report.ts +++ b/tools/content-node/report.ts @@ -113,10 +113,11 @@ function send(mapping: string, info: ReportInfo, body: ReportTerm | ReportTerm[] signal: AbortSignal.timeout(REPORT_TIMEOUT_MS), }) .then(async (response) => { - // Nothing in the answer is read; unread, it would hold the connection. - await response.body?.cancel(); if (response.status !== 200) console.error(`[REP001] The report service answered ${response.status}; no report made`); + // Nothing in the answer is read; unread, it would hold the connection. + // A body already broken has nothing to let go. + await response.body?.cancel().catch(() => {}); }) .catch(() => console.error('[REP002] Report service is unavailable')); } diff --git a/tools/content-node/service.ts b/tools/content-node/service.ts index 7a1a8a4b..fbd26795 100644 --- a/tools/content-node/service.ts +++ b/tools/content-node/service.ts @@ -3497,9 +3497,10 @@ export const endpoints: Endpoint[] = [ status: 200, type: 'application/json', // numFound > 0 with no documents cannot happen at Solr's default rows. - // If it did, this throws and fromSolr answers its Solr failure -- not - // Java's NullPointerException answer, and never a 200 with no body. - body: present(present(searchResult(answer, false).entries)[0]), + // If it did, searchResult leaves `entries` out, this throws, and + // fromSolr answers its Solr failure -- not Java's NullPointerException + // answer, as it did before it was typed. + body: present(searchResult(answer, false).entries)[0], }; }); }, diff --git a/tools/content-node/solr.ts b/tools/content-node/solr.ts index 4b2a385b..8f261709 100644 --- a/tools/content-node/solr.ts +++ b/tools/content-node/solr.ts @@ -191,7 +191,7 @@ export async function solr( if (!response.ok) { // Its body unread holds the connection until collected: one request // that Solr refuses (a query it cannot parse) is enough to ask it. - await response.body?.cancel(); + await response.body?.cancel().catch(() => {}); throw Object.assign(new Error(`Solr answered ${response.status}`), { solr: true }); } // What the handler sent (JSON's null too); the readers check each part they use. diff --git a/tools/content-node/unread-answers.spec.ts b/tools/content-node/unread-answers.spec.ts index a133f0a1..1c85edbe 100644 --- a/tools/content-node/unread-answers.spec.ts +++ b/tools/content-node/unread-answers.spec.ts @@ -63,4 +63,30 @@ describe('an answer nothing reads', () => { await vi.waitFor(() => expect(body.cancelled).toBe(true)); } }); + + it('still says what the report service answered when its body is already broken', async () => { + process.env['REPORT_URL'] = 'http://127.0.0.1:1'; + const broken = new ReadableStream({ + start(controller) { + controller.error(new Error('terminated')); + }, + }); + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(broken, { status: 500 })) + ); + const logged: string[] = []; + vi.spyOn(console, 'error').mockImplementation((line: string) => logged.push(line)); + sendReport( + 'notfound', + { agent: null, ip: '203.0.113.1', release: '97' }, + { + term: 'XYZ', + resource: '', + } + ); + await vi.waitFor(() => expect(logged).toHaveLength(1)); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(logged).toEqual(['[REP001] The report service answered 500; no report made']); + }); });