From ac0cc142fa31cf7afe20bf44584a8b5691f0f9e3 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 4 Jan 2026 09:40:06 -0500 Subject: [PATCH 1/3] fix: add error handling wrapper for graphqlUploadExpress middleware MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The graphqlUploadExpress middleware was returning 500 errors without detailed error messages when file upload parsing failed. This wraps the middleware to catch errors and return proper GraphQL-formatted error responses with helpful messages. Fixes #11293 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- server/routes.ts | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/server/routes.ts b/server/routes.ts index 08bfb9b77fb..fea905fc0a7 100644 --- a/server/routes.ts +++ b/server/routes.ts @@ -307,7 +307,31 @@ export default async (app: express.Application) => { }, }; - app.use('/graphql', graphqlUploadExpress()); + // Wrap graphqlUploadExpress with error handling to provide better error messages + // for multipart upload parsing failures (see https://github.com/opencollective/opencollective-api/issues/11293) + app.use('/graphql', (req, res, next) => { + const uploadMiddleware = graphqlUploadExpress(); + uploadMiddleware(req, res, (err?: Error) => { + if (err) { + logger.error(`GraphQL upload error: ${err.message}`, { error: err }); + // Return a proper JSON error response instead of letting Express error handler return a generic 500 + return res.status(400).json({ + errors: [ + { + message: `File upload failed: ${err.message}`, + extensions: { + code: 'BAD_REQUEST', + exception: { + name: err.name, + }, + }, + }, + ], + }); + } + next(); + }); + }); /** * GraphQL v1 From 8168cb7ae5b57d2543c8633e44fe439b9a2ff7a1 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 4 Jan 2026 09:53:40 -0500 Subject: [PATCH 2/3] test: add TDD test for GraphQL multipart upload error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test verifies that malformed multipart requests return errors in GraphQL format (errors array) rather than Express format (error object). Without the fix: - Returns: { error: { message: '...', code: 400 } } - Test fails: expected undefined to exist (no 'errors' field) With the fix: - Returns: { errors: [{ message: 'File upload failed: ...' }] } - Test passes: GraphQL format is properly returned 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- test/server/routes/graphql.test.ts | 45 ++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/server/routes/graphql.test.ts b/test/server/routes/graphql.test.ts index b717d2e0c28..9d91b0cbbbe 100644 --- a/test/server/routes/graphql.test.ts +++ b/test/server/routes/graphql.test.ts @@ -219,3 +219,48 @@ describe('GraphQL Armor Protection Tests', () => { }); }); }); + +describe('GraphQL Multipart Upload Error Handling', () => { + let app; + + before(async () => { + app = await startTestServer(); + }); + + after(async () => { + await stopTestServer(); + }); + + describe('/graphql/v2', () => { + it('should return errors in GraphQL format for malformed multipart requests', async () => { + // Send a multipart request with invalid JSON in the operations field. + // This triggers an error in graphqlUploadExpress middleware. + // See: https://github.com/opencollective/opencollective-api/issues/11293 + const boundary = '----WebKitFormBoundary7MA4YWxkTrZu0gW'; + const body = [ + `--${boundary}`, + 'Content-Disposition: form-data; name="operations"', + '', + '{ invalid json }', // Invalid JSON triggers parsing error in graphqlUploadExpress + `--${boundary}`, + 'Content-Disposition: form-data; name="map"', + '', + '{}', + `--${boundary}--`, + ].join('\r\n'); + + const response = await request(app) + .post('/graphql/v2') + .set('Content-Type', `multipart/form-data; boundary=${boundary}`) + .set('Authorization', 'Bearer test-token') + .send(body); + + // The error should be returned in GraphQL format (errors array) not Express format (error object) + // This is important because GraphQL clients expect the standard errors array format. + expect(response.status).to.eq(400); + expect(response.body.errors).to.exist; + expect(response.body.errors).to.be.an('array'); + expect(response.body.errors[0].message).to.include('File upload failed'); + }); + }); +}); From ecc3f02da5cb0152bc41e542b759bd17c4900184 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 4 Jan 2026 10:03:51 -0500 Subject: [PATCH 3/3] test: add test for valid multipart upload returning GraphQL format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verifies that valid multipart uploads return GraphQL-formatted responses (not 500 errors) even when authentication fails. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- test/server/routes/graphql.test.ts | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/server/routes/graphql.test.ts b/test/server/routes/graphql.test.ts index 9d91b0cbbbe..5acee84d899 100644 --- a/test/server/routes/graphql.test.ts +++ b/test/server/routes/graphql.test.ts @@ -262,5 +262,42 @@ describe('GraphQL Multipart Upload Error Handling', () => { expect(response.body.errors).to.be.an('array'); expect(response.body.errors[0].message).to.include('File upload failed'); }); + + it('should return GraphQL response (not 500) for valid multipart upload requests', async () => { + // Send a valid multipart upload request following GraphQL multipart spec. + // Even if auth fails, the response should be in GraphQL format, not a raw 500. + // See: https://github.com/opencollective/opencollective-api/issues/11293 + const response = await request(app) + .post('/graphql/v2') + .set('Authorization', 'Bearer test-token') + .field( + 'operations', + JSON.stringify({ + query: `mutation UploadFile($files: [UploadFileInput!]!) { + uploadFile(files: $files) { file { id url name } } + }`, + variables: { files: [{ kind: 'EXPENSE_ITEM', file: null }] }, + }), + ) + .field('map', JSON.stringify({ '0': ['variables.files.0.file'] })) + .attach('0', Buffer.from('test file content'), { + filename: 'test.txt', + contentType: 'text/plain', + }); + + // Should NOT return 500 - should return proper GraphQL response + // Even if auth fails (401-ish error), it should be in GraphQL format + expect(response.status).to.not.eq(500); + + // If there's an error, it should be in GraphQL format + if (response.body.errors) { + expect(response.body.errors).to.be.an('array'); + expect(response.body.errors[0]).to.have.property('message'); + } + // If successful, it should have data + if (response.body.data) { + expect(response.body.data).to.have.property('uploadFile'); + } + }); }); });