Skip to content

Commit 71d3974

Browse files
kckernclaude
andcommitted
fix(web): a search searchd cannot parse shows 0 results, not a 500
Staging traffic parity: /).split( returned 500 because Sphinx rejected the query with ER_PARSE_ERROR. PHP ran the same query through the mysql CLI, got no rows and showed 0 results titled Home. Do the same. Also allow the intended highlight difference for queries with leading or trailing punctuation (/_ignition), limited by a check to pages where only the searchterm spans differ. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 468a6ab commit 71d3974

5 files changed

Lines changed: 51 additions & 2 deletions

File tree

‎apps/web/src/data/external/sphinx.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,15 @@ export class SphinxClient {
6363
}
6464

6565
async search(query, opts) {
66-
const rows = await this.query(buildSearchSql(query, opts));
66+
let rows;
67+
try {
68+
rows = await this.query(buildSearchSql(query, opts));
69+
} catch (err) {
70+
// Operators pass through as typed, so searchd rejects some queries (/).split( ). PHP ran
71+
// them through the mysql CLI, got no rows and showed 0 results.
72+
if (err.code === 'ER_PARSE_ERROR') return [];
73+
throw err;
74+
}
6775
return rows.map(r => ({ verse_id: Number(r.verse_id), version: String(r.version) }));
6876
}
6977

‎apps/web/test/parity/allow.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,5 +13,6 @@
1313
"asset /css/ui.css": "Google sign-in removed: the settings panel's Logout button and its #logout rule are gone (owner decision #11, 2026-10-22)",
1414
"* check:staleSectionHeading": "Live heads a passage whose first verse starts a section with § but not §¶ (a psalm superscription, NIV Ps. 23:1 \"§A psalm of David.¶▼…\", or after a division label, NET Ps. 90:1 \"Book 4 (Psalms 90-106)§A prayer…\") with the section before it (\"Psalm 22\" and an ellipsis on /niv/ps.23); Node heads it with the section at that verse, and a whole chapter that is one section gets section nav as with §¶; a label before §¶ (NIV Ps. 1:1 \"§Psalms 1–41§¶…\") keeps its passage without live's heading (owner decision 2026-10-22). Allowed only where live shows that heading and ellipsis ending right before the first verse, the first verse's live raw text starts a section, Node's heading is the raw heading with the section's range on live's tag, and nothing else differs but the nav labels of the same links",
1515
"latex re:(^|[/;])jst\\. && check:latexMissingVerses": "PHP latex read LDS only and dropped JST verses; Node renders each block's version (decision 2026-10-23). Only for references with a JST piece, and only where every live verse is in Node's output and Node has more (live empty or missing verses)",
16-
"html re:\\d[a-z]$ && check:subverseDigit": "A trailing subverse letter (/john.3.16a): PHP's /\\d[a-z]$/ also dropped the digit before it (John 3:1); Node drops only the letter (requestParsing.js). Allowed where every differing line is Node's with one digit removed"
16+
"html re:\\d[a-z]$ && check:subverseDigit": "A trailing subverse letter (/john.3.16a): PHP's /\\d[a-z]$/ also dropped the digit before it (John 3:1); Node drops only the letter (requestParsing.js). Allowed where every differing line is Node's with one digit removed",
17+
"html re:^/(?:search/)?[^A-Za-z0-9/?][^/?]*$|^/(?:search/)?[^/?]*[^A-Za-z0-9/?.]$ && check:highlightOnly": "Search terms with leading or trailing punctuation (/_ignition): live's empty preg_split piece wraps every word in empty searchterm spans; Node highlights only the query words (intended, searchService.test.js). Allowed where only the highlight spans differ"
1718
}

‎apps/web/test/parity/allow.mjs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@ const CHECKS = {
3030
const oneDigitLess = (a, b) => b.length === a.length + 1 && [...b].some((ch, i) => /\d/.test(ch) && b.slice(0, i) + b.slice(i + 1) === a);
3131
return l.some((line, i) => line !== n[i]) && l.every((line, i) => line === n[i] || oneDigitLess(line, n[i]));
3232
},
33+
// Only the search highlight spans differ: without them, the same markup lines in order and the
34+
// same text (quoted lines joined, whitespace ignored)
35+
highlightOnly: ({ live, node }) => {
36+
if (typeof live !== 'string' || typeof node !== 'string') return false;
37+
const strip = body => {
38+
const lines = body.split('\n').map(l => l.trim()).filter(l => l !== '<span class="searchterm">');
39+
return {
40+
markup: lines.filter(l => !l.startsWith('"')).join('\n'),
41+
text: lines.filter(l => l.startsWith('"')).map(l => l.replace(/^"|"$/g, '')).join('').replace(/\s+/g, ''),
42+
};
43+
};
44+
const l = strip(live), n = strip(node);
45+
return l.markup === n.markup && l.text === n.text;
46+
},
3347
// PHP latex read the request's version only and dropped verses (JST) Node prints: every live
3448
// verse is in Node's output, and Node has more
3549
latexMissingVerses: ({ live, node }) => {

‎apps/web/test/parity/allow.test.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,19 @@ describe('allow.json: a trailing subverse letter keeps the digit before it', ()
140140
expect(reason('/xx386', page(''), page('6'))).toBeUndefined();
141141
});
142142
});
143+
144+
describe('allow.json: highlight spans for a query with leading or trailing punctuation', () => {
145+
const allow = JSON.parse(readFileSync(new URL('./allow.json', import.meta.url), 'utf8'));
146+
const span = ' <span class="searchterm">';
147+
const live = ['<li>', span, ' "The"', span, '"more ', span, ' "igniting"', '</li>'].join('\n');
148+
const node = ['<li>', ' "The more "', span, ' "igniting"', '</li>'].join('\n');
149+
const reason = (path, l, n) => allowReason(allow, 'html', path, l, { live: l, node: n });
150+
test('allowed where only the highlight spans differ', () => {
151+
expect(reason('/_ignition', live, node)).toMatch(/punctuation/);
152+
});
153+
test('not where the text or markup differs, nor for a plain query', () => {
154+
expect(reason('/_ignition', live, node.replace('more', 'less'))).toBeUndefined();
155+
expect(reason('/_ignition', live, node.replace('</li>', '</ul>'))).toBeUndefined();
156+
expect(reason('/ignition', live, node)).toBeUndefined();
157+
});
158+
});

‎apps/web/test/unit/sphinx.test.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,16 @@ describe('SphinxClient', () => {
7575
expect(connect).toHaveBeenCalledTimes(2);
7676
});
7777

78+
test('a query searchd cannot parse finds nothing, as live does', async () => {
79+
// /).split( : PHP ran the query through the mysql CLI, got no rows on the syntax error and
80+
// showed 0 results
81+
const bad = Object.assign(new Error("index 'sgindex': syntax error, unexpected ')'"), { code: 'ER_PARSE_ERROR' });
82+
const connect = vi.fn(async () => ({ query: vi.fn(async () => { throw bad; }), end: vi.fn(async () => {}) }));
83+
const client = new SphinxClient({ host: 'sphinx' }, connect);
84+
await expect(client.search(') split(')).resolves.toEqual([]);
85+
await expect(client.query('SELEC')).rejects.toThrow('syntax error');
86+
});
87+
7888
test('createSphinxClient is null without a host', () => {
7989
expect(createSphinxClient({ host: '' })).toBeNull();
8090
expect(createSphinxClient({ host: 'sphinx', port: 9306 })).toBeInstanceOf(SphinxClient);

0 commit comments

Comments
 (0)