Skip to content

Commit

Permalink
Merge pull request #382 from Inist-CNRS/fix-error-body-lost2
Browse files Browse the repository at this point in the history
fix: 🐛 invalid header with error
  • Loading branch information
touv authored Nov 10, 2023
2 parents 26747cf + bee412f commit 9a03736
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 10 deletions.
1 change: 1 addition & 0 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"from": "0.1.7",
"global-modules": "2.0.0",
"http-shutdown": "1.2.2",
"json-buffer": "3.0.1",
"load-json-file": "6.2.0",
"lodash": "4.17.21",
"lru-cache": "5.1.1",
Expand Down
6 changes: 4 additions & 2 deletions packages/core/src/server/errorHandler.js
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import debug from 'debug';
import JSONB from 'json-buffer';
import { httpRequestErrorTotal } from './metrics';

const errorHandler = (request, response) => (error, code = 400) => {
debug('ezs')('Server has caught an error', error);
debug('ezs')('Server has caught an error', code, error);
httpRequestErrorTotal.labels(request.pathName).inc();
if (response.headersSent) {
return response.end();
}
const bodyResponse = JSON.stringify(error);
const bodyResponse = JSONB.stringify(error);
response.writeHead(code, {
'Content-Type': 'application/json',
'Content-Length': bodyResponse.length,
'Content-Encoding': 'identity',
});
return response.end(bodyResponse);
};
Expand Down
16 changes: 11 additions & 5 deletions packages/core/src/server/knownPipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,30 @@ const knownPipeline = (ezs) => (request, response, next) => {
.pipe(ezs('truncate', { length: request.headers['content-length'] }))
.pipe(ezs.uncompress(request.headers));

const outputStream = new PassThrough();
outputStream.pipe(response);
const transformedStream = ezs.createPipeline(decodedStream, statements)
.pipe(ezs.catch((e) => e))
.on('error', (e) => {
outputStream.unpipe(response);
responseStarted();
triggerError(e, 400);
rawStream.destroy();
decodedStream.destroy();
transformedStream.destroy();
responseStarted();
triggerError(e, 400);
});

pipeline(
transformedStream,
ezs.toBuffer(),
ezs.compress(response.getHeaders()),
response,
outputStream,
(e) => {
responseStarted();
triggerError(e, 500);
if (e) {
outputStream.unpipe(response);
responseStarted();
triggerError(e, 500);
}
}
);

Expand Down
18 changes: 15 additions & 3 deletions packages/core/test/knownPipeline.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ describe(' through server(s)', () => {
const stream = from(input);
fetch('http://127.0.0.1:33333/buggy1.ini', { method: 'POST', body: stream })
.then((res) => {
assert(res.headers.has('x-error'));
assert(!res.ok);
return res.json();
})
.then((json) => {
assert.equal(json.scope, 'statements');
done();
})
.catch(done);
Expand All @@ -98,7 +102,11 @@ describe(' through server(s)', () => {
const stream = from(input);
fetch('http://127.0.0.1:33333/buggy2.ini', { method: 'POST', body: stream })
.then((res) => {
assert(res.headers.has('x-error'));
assert(!res.ok);
return res.json();
})
.then((json) => {
assert.equal(json.scope, 'data');
done();
})
.catch(done);
Expand All @@ -109,7 +117,11 @@ describe(' through server(s)', () => {
const stream = from(input);
fetch('http://127.0.0.1:33333/buggy3.ini', { method: 'POST', body: stream })
.then((res) => {
assert(res.headers.has('x-error'));
assert(!res.ok);
return res.json();
})
.then((json) => {
assert.equal(json.scope, 'statements');
done();
})
.catch(done);
Expand Down

0 comments on commit 9a03736

Please sign in to comment.