streamErrorHandler: write error trailer before closing stream (#922)

getodk · Jul 26, 2023 · 0c16bb7 · 0c16bb7
1 parent a381927
commit 0c16bb7
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 8 deletions.
diff --git a/lib/http/endpoint.js b/lib/http/endpoint.js
@@ -23,7 +23,7 @@ const { openRosaError } = require('../formats/openrosa');
 const { odataXmlError } = require('../formats/odata');
 const { noop, isPresent } = require('../util/util');
 const { serialize, redirect } = require('../util/http');
-const { resolve, reject, rejectIfError } = require('../util/promise');
+const { resolve, reject } = require('../util/promise');
 const { PartialPipe } = require('../util/stream');
 const Problem = require('../util/problem');
 
@@ -188,18 +188,18 @@ const endpointBase = ({ preprocessor = noop, before = noop, resultWriter, errorW
 ////////////////////////////////////////
 // JSON/default
 
-const streamErrorHandler = (response, next) => (err) => {
+const streamErrorHandler = (response) => () => {
   response.addTrailers({ Status: 'Error' }); // TODO: improve response content.
-  response.end();
-  next(err);
 };
 const defaultResultWriter = (result, request, response, next) => {
   if (!response.hasHeader('Content-Type')) response.type('json');
 
   if (result instanceof PartialPipe) {
-    result.with(response).pipeline(streamErrorHandler(response, next));
+    result.streams.at(-1).on('error', streamErrorHandler(response));
+    result.with(response).pipeline((err) => next?.(err));
   } else if (result.pipe != null) {
-    pipeline(result, response, rejectIfError(streamErrorHandler(response, next)));
+    result.on('error', streamErrorHandler(response));
+    pipeline(result, response, (err) => next?.(err));
   } else {
     response.send(serialize(result));
   }

diff --git a/test/unit/http/endpoint.js b/test/unit/http/endpoint.js
@@ -499,7 +499,8 @@ describe('endpoints', () => {
       let trailers;
       const requestTest = streamTest.fromChunks();
       // eslint-disable-next-line no-shadow
-      const responseTest = streamTest.toText((_, result) => {
+      const responseTest = streamTest.toText((err, result) => {
+        err.message.should.equal('ERR_EXPECTED');
         trailers.should.eql({ Status: 'Error' });
         // eslint-disable-next-line no-multi-spaces
         should.not.exist(result);                  // node v14
@@ -515,7 +516,7 @@ describe('endpoints', () => {
         streamTest.fromChunks([ 'a', 'test', 'stream' ]),
         // eslint-disable-next-line no-shadow
         new Transform({ transform(s, _, done) {
-          if (s.length > 4) done(new Error('nope'));
+          if (s.length > 4) done(new Error('ERR_EXPECTED'));
           else done(null, s + '!');
         } })
       );