self-hosted/joplin
1
0
Fork 0
mirror of https://github.com/laurent22/joplin.git synced 2025-08-24 20:19:10 +02:00
Code Issues Releases Activity

Server: Fixes #12983: Not handling correctly non JSON error responses from Transcribe (#12986)

Browse Source
This commit is contained in:
pedr
2025-08-20 03:46:15 -03:00
committed by GitHub
parent 26012cd7d5
commit f1d452f130
2 changed files with 90 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

126
packages/server/src/routes/api/transcribe.test.ts
View File

@@ -18,6 +18,23 @@ type JobWithResult = {
state: string;
};
type TranscribeJobResponse = {
jobId: string;
};
type TranscribeResponse = TranscribeJobResponse | JobWithResult | string;
type FetchResponse = {
json?: ()=> Promise<TranscribeResponse>;
text?: ()=> Promise<string>;
status: number;
};
const fetchSpyOn = (response: FetchResponse) => {
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(response)) as jest.Mock,
);
};
describe('api_transcribe', () => {
@@ -41,16 +58,12 @@ describe('api_transcribe', () => {
test('should create job', async () => {
const { session } = await createUserAndSession(1);
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(
{
json: () => Promise.resolve(
{ jobId: '608626f1-cad9-4b07-a02e-ec427c47147f' },
),
status: 200,
})) as jest.Mock,
);
fetchSpyOn({
json: () => Promise.resolve(
{ jobId: '608626f1-cad9-4b07-a02e-ec427c47147f' },
),
status: 200,
});
const fileContent = await readFile(`${testAssetDir}/htr_example.png`);
const tempFilePath = await makeTempFileWithContent(fileContent);
const response = await postApi<TranscribeJob>(session.id, 'transcribe', {},
@@ -65,15 +78,12 @@ describe('api_transcribe', () => {
test('should create job and return response eventually', async () => {
const { session } = await createUserAndSession(1);
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(
{
json: () => Promise.resolve(
{ jobId: '608626f1-cad9-4b07-a02e-ec427c47147f' },
),
status: 200,
})) as jest.Mock,
);
fetchSpyOn({
json: () => Promise.resolve(
{ jobId: '608626f1-cad9-4b07-a02e-ec427c47147f' },
),
status: 200,
});
const fileContent = await readFile(`${testAssetDir}/htr_example.png`);
const tempFilePath = await makeTempFileWithContent(fileContent);
@@ -85,19 +95,16 @@ describe('api_transcribe', () => {
expect(postResponse.jobId).not.toBe(undefined);
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(
fetchSpyOn({
json: () => Promise.resolve(
{
json: (): Promise<JobWithResult> => Promise.resolve(
{
id: '608626f1-cad9-4b07-a02e-ec427c47147f',
state: 'completed',
result: { result: 'transcription' },
},
),
status: 200,
})) as jest.Mock,
);
id: '608626f1-cad9-4b07-a02e-ec427c47147f',
state: 'completed',
result: { result: 'transcription' },
},
),
status: 200,
});
const getResponse = await getApi<JobWithResult>(session.id, `transcribe/${postResponse.jobId}`, {});
expect(getResponse.id).toBe(postResponse.jobId);
@@ -108,13 +115,10 @@ describe('api_transcribe', () => {
test('should throw a error if API returns error 400', async () => {
const { session } = await createUserAndSession(1);
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(
{
json: () => Promise.resolve(''),
status: 400,
})) as jest.Mock,
);
fetchSpyOn({
json: () => Promise.resolve(''),
status: 400,
});
const fileContent = await readFile(`${testAssetDir}/htr_example.png`);
const tempFilePath = await makeTempFileWithContent(fileContent);
@@ -131,13 +135,10 @@ describe('api_transcribe', () => {
test('should throw error if API returns error 500', async () => {
const { session } = await createUserAndSession(1);
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(
{
json: () => Promise.resolve(''),
status: 500,
})) as jest.Mock,
);
fetchSpyOn({
json: () => Promise.resolve(''),
status: 500,
});
const fileContent = await readFile(`${testAssetDir}/htr_example.png`);
const tempFilePath = await makeTempFileWithContent(fileContent);
@@ -153,13 +154,10 @@ describe('api_transcribe', () => {
test('should throw 500 error is something unexpected', async () => {
const { session } = await createUserAndSession(1);
jest.spyOn(global, 'fetch').mockImplementation(
jest.fn(() => Promise.resolve(
{
json: () => Promise.reject(new Error('Something went wrong')),
status: 200,
})) as jest.Mock,
);
fetchSpyOn({
json: () => Promise.reject(new Error('Something went wrong')),
status: 200,
});
const fileContent = await readFile(`${testAssetDir}/htr_example.png`);
const tempFilePath = await makeTempFileWithContent(fileContent);
@@ -174,4 +172,28 @@ describe('api_transcribe', () => {
expect(error.message.startsWith('POST /api/transcribe {"status":500,"body":{"error":"Something went wrong"')).toBe(true);
});
test.each([
['non-json', 'Something went wrong'],
['json', JSON.stringify({ error: 'Something went wrong' })],
])('should be able to handle %s error responses', async (_type: string, result: string) => {
const { session } = await createUserAndSession(1);
fetchSpyOn({
text: () => Promise.resolve(result),
status: 500,
});
const fileContent = await readFile(`${testAssetDir}/htr_example.png`);
const tempFilePath = await makeTempFileWithContent(fileContent);
const error = await expectThrow(() =>
postApi<TranscribeJob>(session.id, 'transcribe', {},
{
filePath: tempFilePath,
},
));
const body = JSON.parse(error.message.split('POST /api/transcribe ')[1]);
expect(error.httpCode).toBe(502);
expect(body.body.error).toBe('Something went wrong');
});
});

20
packages/server/src/routes/api/transcribe.ts
View File

@@ -17,6 +17,18 @@ const isHtrSupported = () => {
return config().TRANSCRIBE_ENABLED;
};
const parseResponseSafely = async (response: Response) => {
const text = await response.text();
try {
return JSON.parse(text);
} catch (parseError) {
const truncatedText = text.substring(0, 1000);
return { error: truncatedText };
}
};
router.get('api/transcribe/:id', async (path: SubPath, _ctx: AppContext) => {
if (!isHtrSupported()) {
throw new ErrorNotImplemented('HTR feature is not enabled in this server');
@@ -36,8 +48,8 @@ router.get('api/transcribe/:id', async (path: SubPath, _ctx: AppContext) => {
const responseJson = await response.json();
throw new ErrorBadRequest(responseJson.error);
} else if (response.status >= 500) {
const responseJson = await response.json();
throw new ErrorBadGateway(responseJson.error);
const responseParsed = await parseResponseSafely(response);
throw new ErrorBadGateway(responseParsed.error);
}
const responseJson = await response.json();
@@ -77,8 +89,8 @@ router.post('api/transcribe', async (_path: SubPath, ctx: AppContext) => {
const responseJson = await response.json();
throw new ErrorBadRequest(responseJson.error);
} else if (response.status >= 500) {
const responseJson = await response.json();
throw new ErrorBadGateway(responseJson.error);
const responseParsed = await parseResponseSafely(response);
throw new ErrorBadGateway(responseParsed.error);
}
const responseJson = await response.json();