Skip to content

Commit b5704e8

Browse files
Harden Noswhere translation response handling and add failure diagnostics (#36)
Agent-Logs-Url: https://github.com/damus-io/api/sessions/30936094-ca55-465a-9ffa-c7f7bd2d4a58 Co-authored-by: danieldaquino <24692108+danieldaquino@users.noreply.github.com>
1 parent 6728f22 commit b5704e8

2 files changed

Lines changed: 116 additions & 5 deletions

File tree

src/translate/noswhere.js

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,27 @@ module.exports = class NoswhereTranslator {
77
constructor() {
88
if (!this.#noswhereKey)
99
throw new Error("expected NOSWHERE_KEY env var")
10-
this.#loadTranslationLangs()
10+
this.#loadTranslationLangs().catch((err) => {
11+
console.error("error loading noswhere translation langs: %o", err)
12+
})
13+
}
14+
#getRequestId(resp) {
15+
return resp.headers?.get?.("x-noswhere-request") || "unknown"
16+
}
17+
#getBodySnippet(body) {
18+
if (typeof body !== "string" || body.length === 0) return "<empty>"
19+
if (body.length <= 500) return body
20+
return body.slice(0, 500) + "...(truncated)"
21+
}
22+
async #parseResponse(resp, action) {
23+
const requestId = this.#getRequestId(resp)
24+
const body = await resp.text()
25+
try {
26+
return JSON.parse(body)
27+
} catch (err) {
28+
console.error("noswhere %s response parse error: status=%s ok=%s request=%s body=%o", action, resp.status, resp.ok, requestId, this.#getBodySnippet(body))
29+
throw new Error(`error ${action}: invalid JSON response from Noswhere (request: ${requestId})`)
30+
}
1131
}
1232
async #loadTranslationLangs() {
1333
let resp = await fetch(this.#noswhereURL + "/langs", {
@@ -18,9 +38,9 @@ module.exports = class NoswhereTranslator {
1838
'Content-Type': 'application/json'
1939
}
2040
})
21-
let data = await resp.json()
41+
let data = await this.#parseResponse(resp, "getting translation langs")
2242
if (!resp.ok) {
23-
throw new Error(`error getting translation langs: API failed with ${resp.status} ${data.error} (request: ${resp.headers.get("x-noswhere-request")})`)
43+
throw new Error(`error getting translation langs: API failed with ${resp.status} ${data.error} (request: ${this.#getRequestId(resp)})`)
2444
}
2545
if (!data[this.#type]) {
2646
throw new Error(`type ${this.#type} not supported for translation`)
@@ -46,9 +66,9 @@ module.exports = class NoswhereTranslator {
4666
})
4767
})
4868

49-
let data = await resp.json()
69+
let data = await this.#parseResponse(resp, "translating")
5070
if (!resp.ok) {
51-
throw new Error(`error translating: API failed with ${resp.status} ${data.error} (request: ${resp.headers.get("x-noswhere-request")})`)
71+
throw new Error(`error translating: API failed with ${resp.status} ${data.error} (request: ${this.#getRequestId(resp)})`)
5272
}
5373

5474
if (data.result) {

test/noswhere.test.js

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
const tap = require('tap');
2+
const sinon = require('sinon');
3+
4+
const NOSWHERE_TRANSLATOR_PATH = require.resolve('../src/translate/noswhere.js');
5+
6+
function loadNoswhereTranslator() {
7+
delete require.cache[NOSWHERE_TRANSLATOR_PATH];
8+
return require(NOSWHERE_TRANSLATOR_PATH);
9+
}
10+
11+
function mockHeaders(requestId) {
12+
return {
13+
get: (name) => name === 'x-noswhere-request' ? requestId : null,
14+
};
15+
}
16+
17+
tap.test('NoswhereTranslator constructor logs malformed langs responses without crashing', async (t) => {
18+
process.env.NOSWHERE_KEY = 'test-key';
19+
20+
const fetchStub = sinon.stub(global, 'fetch').resolves({
21+
ok: true,
22+
status: 200,
23+
headers: mockHeaders('langs-req-1'),
24+
text: async () => '',
25+
});
26+
const errorStub = sinon.stub(console, 'error');
27+
28+
t.teardown(() => {
29+
fetchStub.restore();
30+
errorStub.restore();
31+
delete process.env.NOSWHERE_KEY;
32+
delete require.cache[NOSWHERE_TRANSLATOR_PATH];
33+
});
34+
35+
const NoswhereTranslator = loadNoswhereTranslator();
36+
const translator = new NoswhereTranslator();
37+
38+
await new Promise((resolve) => setImmediate(resolve));
39+
40+
t.equal(translator.canTranslate('en', 'ja'), true, 'translator stays usable when langs cannot be loaded');
41+
t.ok(errorStub.calledTwice, 'malformed response is logged without escaping the constructor');
42+
t.match(errorStub.firstCall.args[0], /noswhere %s response parse error/, 'parse failure is logged');
43+
t.match(errorStub.firstCall.args.slice(1).join(' '), /langs-req-1/, 'request id is included in log output');
44+
t.match(errorStub.firstCall.args.slice(1).join(' '), /<empty>/, 'response body snippet is included in log output');
45+
t.match(errorStub.secondCall.args[0], /error loading noswhere translation langs/, 'constructor catch logs the failure summary');
46+
});
47+
48+
tap.test('NoswhereTranslator translate rejects malformed JSON responses with request details', async (t) => {
49+
process.env.NOSWHERE_KEY = 'test-key';
50+
51+
const fetchStub = sinon.stub(global, 'fetch');
52+
fetchStub.onFirstCall().resolves({
53+
ok: true,
54+
status: 200,
55+
headers: mockHeaders('langs-req-2'),
56+
text: async () => JSON.stringify({
57+
default: {
58+
from: ['en'],
59+
to: ['ja'],
60+
},
61+
}),
62+
});
63+
fetchStub.onSecondCall().resolves({
64+
ok: true,
65+
status: 200,
66+
headers: mockHeaders('translate-req-1'),
67+
text: async () => '{"result"',
68+
});
69+
const errorStub = sinon.stub(console, 'error');
70+
71+
t.teardown(() => {
72+
fetchStub.restore();
73+
errorStub.restore();
74+
delete process.env.NOSWHERE_KEY;
75+
delete require.cache[NOSWHERE_TRANSLATOR_PATH];
76+
});
77+
78+
const NoswhereTranslator = loadNoswhereTranslator();
79+
const translator = new NoswhereTranslator();
80+
81+
await new Promise((resolve) => setImmediate(resolve));
82+
83+
await t.rejects(
84+
translator.translate('en', 'ja', 'hello'),
85+
/error translating: invalid JSON response from Noswhere \(request: translate-req-1\)/,
86+
'translate surfaces a helpful parse error'
87+
);
88+
t.equal(errorStub.calledOnce, true, 'translate parse failures are logged');
89+
t.match(errorStub.firstCall.args.slice(1).join(' '), /translate-req-1/, 'translate log includes request id');
90+
t.match(errorStub.firstCall.args.slice(1).join(' '), /{"result"/, 'translate log includes body snippet');
91+
});

0 commit comments

Comments
 (0)