-
Notifications
You must be signed in to change notification settings - Fork 2
fix: add json parse error handler #300
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| import { describe, test, expect } from 'vitest'; | ||
|
|
||
| describe('HTTP API Error Handler', () => { | ||
| describe('JSON Parse Error Handler', () => { | ||
| test('Returns 400 when request body contains not complete JSON', async () => { | ||
| const response = await global.api?.fakeRequest({ | ||
| method: 'POST', | ||
| url: '/join/test-hash1', | ||
| headers: { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: '{invalid json', | ||
| }); | ||
|
|
||
| expect(response?.statusCode).toBe(400); | ||
|
|
||
| const body = await response?.json(); | ||
|
|
||
| expect(body).toStrictEqual({ | ||
| message: 'Invalid JSON in request body', | ||
| }); | ||
| }); | ||
|
|
||
| test('Returns 400 when request body contains malformed JSON', async () => { | ||
| const response = await global.api?.fakeRequest({ | ||
| method: 'POST', | ||
| url: '/join/test-hash1', | ||
| headers: { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: '{"key": "value",}', | ||
| }); | ||
|
|
||
| expect(response?.statusCode).toBe(400); | ||
|
|
||
| const body = await response?.json(); | ||
|
|
||
| expect(body).toStrictEqual({ | ||
| message: 'Invalid JSON in request body', | ||
| }); | ||
| }); | ||
|
|
||
| test('Returns 400 when JSON body is empty', async () => { | ||
| const response = await global.api?.fakeRequest({ | ||
| method: 'POST', | ||
| url: '/join/test-hash1', | ||
| headers: { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: '', | ||
| }); | ||
|
|
||
| expect(response?.statusCode).toBe(400); | ||
|
|
||
| const body = await response?.json(); | ||
|
|
||
| expect(body).toStrictEqual({ | ||
| message: 'Invalid JSON in request body', | ||
| }); | ||
| }); | ||
|
|
||
| test('Does not return 400 for valid JSON', async () => { | ||
| const response = await global.api?.fakeRequest({ | ||
| method: 'POST', | ||
| url: '/join/test-hash1', | ||
| headers: { | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: '{"key": "value"}', | ||
| }); | ||
|
|
||
| expect(response?.statusCode).not.toBe(400); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,11 @@ import UploadRouter from './router/upload.js'; | |
| import { ajvFilePlugin } from '@fastify/multipart'; | ||
| import { UploadSchema } from './schema/Upload.js'; | ||
| import { NoteHierarchySchema } from './schema/NoteHierarchy.js'; | ||
| import { StatusCodes } from 'http-status-codes'; | ||
|
|
||
| interface FastifyError extends Error { | ||
| code: string; | ||
| } | ||
|
|
||
| const appServerLogger = getLogger('appServer'); | ||
|
|
||
|
|
@@ -372,6 +377,21 @@ export default class HttpApi implements Api { | |
|
|
||
| return; | ||
| } | ||
| /** | ||
| * JSON parse errors (invalid request body) | ||
| * Errors can be either SyntaxError or FastifyError. | ||
| */ | ||
| if ((error instanceof SyntaxError && error.message.includes('JSON')) | ||
| || ((error as FastifyError).code?.startsWith('FST_ERR_CTP_') ?? false)) { | ||
| this.log.warn({ reqId: request.id }, 'Invalid JSON in request body'); | ||
|
|
||
| return reply | ||
| .code(StatusCodes.BAD_REQUEST) | ||
| .type('application/json') | ||
| .send({ | ||
| message: 'Invalid JSON in request body', | ||
| }); | ||
|
Comment on lines
+380
to
+393
|
||
| } | ||
| /** | ||
| * If error is not a domain error, we route it to the default error handler | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is suppose that this type of errors aren't related to domain errors at all => this type of error handlers should be moved outside of the
domainErrorHandler