End asyncStream later in the process (before piping) (#185)

* End asyncStream later in the process (before piping) * Address comments + add test (would fail without without the new changes)
zalando · Sep 22, 2017 · b253598 · b253598
1 parent 2eba01e
commit b253598
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 3 deletions.
diff --git a/lib/request-handler.js b/lib/request-handler.js
@@ -78,6 +78,10 @@ module.exports = function processRequest(options, request, response) {
     } = options;
 
     const asyncStream = new AsyncStream();
+    asyncStream.once('plugged', () => {
+        asyncStream.end();
+    });
+
     const contextPromise = fetchContext(request).catch(err => {
         this.emit('context:error', request, err);
         return {};
@@ -195,9 +199,6 @@ module.exports = function processRequest(options, request, response) {
             });
 
             resultStream.once('finish', () => {
-                // Flush the async stream only after stringifer stream is finished
-                // This guarentees the async fragments are flushed to the client at last
-                asyncStream.end();
                 const statusCode = response.statusCode || 200;
                 if (shouldWriteHead) {
                     shouldWriteHead = false;

diff --git a/lib/streams/stringifier-stream.js b/lib/streams/stringifier-stream.js
@@ -1,5 +1,6 @@
 'use strict';
 const stream = require('stream');
+const AsyncStream = require('./async-stream');
 
 module.exports = class StringifierStream extends stream.Transform {
     constructor(fn) {
@@ -22,6 +23,10 @@ module.exports = class StringifierStream extends stream.Transform {
         }
         let st = this.queue.shift();
         if (st instanceof stream) {
+            if (st instanceof AsyncStream) {
+                st.emit('plugged');
+            }
+
             st.setMaxListeners(st.getMaxListeners() + 1);
             this.isBusy = true;
 

diff --git a/tests/tailor.js b/tests/tailor.js
@@ -6,6 +6,7 @@ const sinon = require('sinon');
 const zlib = require('zlib');
 const { TEMPLATE_NOT_FOUND } = require('../lib/fetch-template');
 const Tailor = require('../index');
+const processTemplate = require('../lib/process-template');
 
 describe('Tailor', () => {
     let server;
@@ -56,6 +57,24 @@ describe('Tailor', () => {
                     return Promise.reject(error);
                 }
             },
+            handledTags: ['delayed-fragment'],
+            handleTag: (request, tag, options, context) => {
+                if (tag.name === 'delayed-fragment') {
+                    const st = processTemplate(request, options, context);
+                    setTimeout(() => {
+                        st.end({
+                            name: 'fragment',
+                            attributes: {
+                                async: true,
+                                src: 'https://fragment/1'
+                            }
+                        });
+                    }, 10);
+                    return st;
+                }
+
+                return '';
+            },
             pipeInstanceName: 'p',
             pipeAttributes: attributes => ({ id: attributes.id }),
             filterResponseHeaders: (attributes, headers) => headers
@@ -645,6 +664,26 @@ describe('Tailor', () => {
         });
     });
 
+    describe('Custom async fragments', () => {
+        it('should add async fragments from handleTag', done => {
+            nock('https://fragment')
+                .get('/1')
+                .reply(200, 'hello');
+
+            mockTemplate.returns('<delayed-fragment></delayed-fragment>');
+            mockChildTemplate.returns('');
+
+            getResponse('http://localhost:8080/test')
+                .then(response => {
+                    assert.equal(
+                        response.body,
+                        '<html><head></head><body><script data-pipe>p.placeholder(0)</script><script data-pipe>p.start(0)</script>hello<script data-pipe>p.end(0)</script></body></html>'
+                    );
+                })
+                .then(done, done);
+        });
+    });
+
     describe('Slots::Tailor ', () => {
         it('should support base templates using slots', done => {
             mockTemplate.returns(