Rework proxy postResponse. Closes hapijs#1247

jmonster · Dec 22, 2013 · 910be37 · 910be37
1 parent 2183d12
commit 910be37
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 62 deletions.
diff --git a/docs/Reference.md b/docs/Reference.md
@@ -449,12 +449,12 @@ The following options are available when adding a route:
                 - `headers` - optional object where each key is an HTTP request header and the value is the header content.
         - `postResponse` - a custom function for processing the response from the upstream service before sending to the client. Useful for
           custom error handling of responses from the proxied endpoint or other payload manipulation. Function signature is
-          `function(request, reply, settings, res, payload, ttl)` where:
+          `function(request, reply, res, settings, ttl)` where:
               - `request` - is the incoming `request` object.
               - `reply()` - the continuation function.
+              - `res` - the node response object received from the upstream service. `res` is a readable stream (use the
+                [**nipple**](https://github.com/spumko/nipple) module `parse` method to easily convert it to a Buffer or string).
               - `settings` - the proxy handler configuration.
-              - `res` - the node response object received from the upstream service.
-              - `payload` - the response payload.
               - `ttl` - the upstream TTL in milliseconds if `proxy.ttl` it set to `'upstream'` and the upstream response included a valid
                 'Cache-Control' header with 'max-age'.
         - `ttl` - if set to `'upstream'`, applies the upstream response caching policy to the response using the `response.ttl()` method (or passed

diff --git a/examples/proxy.js b/examples/proxy.js
@@ -1,5 +1,6 @@
 // Load modules
 
+var Nipple = require('nipple');
 var Hapi = require('../lib');
 
 
@@ -17,6 +18,32 @@ internals.main = function () {
         callback(null, 'http://www.google.com/search?q=' + request.params.term);
     };
 
+    var postResponse = function (request, reply, res, settings, ttl) {
+
+        if (res.statusCode !== 200) {
+            return reply(Boom.badGateway());
+        }
+
+        Nipple.parse(res, function (err, payload) {
+
+            if (err) {
+                return reply(err);
+            }
+
+            var contentType = res.headers['content-type'];
+
+            var response = reply(payload);
+            if (ttl) {
+                response.ttl(ttl);
+            }
+
+            if (contentType) {
+                response.type(contentType);
+            }
+
+        });
+    };
+
     server.route({ method: '*', path: '/{p*}', handler: { proxy: { host: 'google.com', port: 80, redirects: 5 } } });
     server.route({ method: 'GET', path: '/hapi/{term}', handler: { proxy: { mapUri: mapper } } });
     server.start();

diff --git a/lib/proxy.js b/lib/proxy.js
@@ -90,25 +90,15 @@ exports.handler = function (route, options) {
                     }
                 }
 
-                if (!settings.postResponse) {
-                    res._hapi = { passThrough: settings.passThrough };
-                    var response = reply(res);                              // Will pass-through headers and status code
-                    if (ttl) {
-                        response.ttl(ttl);
-                    }
-                    return;
+                if (settings.postResponse) {
+                    return settings.postResponse(request, reply, res, settings, ttl);
                 }
 
-                // Parse payload for caching or post-processing
-
-                Nipple.parse(res, function (err, buffer) {
-
-                    if (err) {
-                        return reply(err);
-                    }
-
-                    return settings.postResponse(request, reply, settings, res, buffer.toString(), ttl);
-                });
+                res._hapi = { passThrough: settings.passThrough };
+                var response = reply(res);                              // Will pass-through headers and status code
+                if (ttl) {
+                    response.ttl(ttl);
+                }
             });
         });
     };
@@ -142,24 +132,3 @@ internals.mapUri = function (protocol, host, port, uri) {
         return next(null, baseUrl + request.path + (request.url.search || ''));
     };
 };
-
-
-/*
-internals.postResponse = function (request, reply, settings, res, payload, ttl) {
-
-    var contentType = res.headers['content-type'];
-
-    if (res.statusCode !== 200) {
-        return reply(Boom.passThrough(res.statusCode, payload, contentType));
-    }
-
-    var response = reply(payload);
-    if (ttl) {
-        response.ttl(ttl);
-    }
-
-    if (contentType) {
-        response.type(contentType);
-    }
-};
-*/
diff --git a/test/integration/proxy.js b/test/integration/proxy.js
@@ -84,7 +84,7 @@ describe('Proxy', function () {
             reply(Hapi.error.unauthorized('Not authorized'));
         };
 
-        var postResponseWithError = function (request, reply, settings, res, payload, ttl) {
+        var postResponseWithError = function (request, reply, res, settings, ttl) {
 
             reply(Hapi.error.forbidden('Forbidden'));
         };
@@ -161,7 +161,6 @@ describe('Proxy', function () {
             { method: 'GET', path: '/redirect', handler: redirectHandler },
             { method: 'POST', path: '/post1', handler: function (request, reply) { reply().redirect('/post2').rewritable(false); } },
             { method: 'POST', path: '/post2', handler: function (request, reply) { reply(request.payload); } },
-            { method: 'GET', path: '/custom', handler: profile },
             { method: 'GET', path: '/timeout1', handler: timeoutHandler },
             { method: 'GET', path: '/timeout2', handler: timeoutHandler },
             { method: 'GET', path: '/handlerOldSchool', handler: activeItem }
@@ -192,9 +191,9 @@ describe('Proxy', function () {
             return callback(null, 'https://127.0.0.1:' + upstreamSingle.info.port);
         };
 
-        var postResponse = function (request, reply, settings, res, payload, ttl) {
+        var postResponse = function (request, reply, res, settings, ttl) {
 
-            reply(payload);
+            reply(res);
         };
 
         upstream.start(function () {
@@ -227,7 +226,6 @@ describe('Proxy', function () {
                         { method: 'GET', path: '/redirect', handler: { proxy: { host: 'localhost', port: backendPort, passThrough: true, redirects: 2 } } },
                         { method: 'POST', path: '/post1', handler: { proxy: { host: 'localhost', port: backendPort, redirects: 3 } }, config: { payload: 'stream' } },
                         { method: 'GET', path: '/nowhere', handler: { proxy: { host: 'no.such.domain.x8' } } },
-                        { method: 'GET', path: '/custom', handler: { proxy: { host: 'localhost', port: backendPort, postResponse: postResponse } }, config: { cache: routeCache } },
                         { method: 'GET', path: '/timeout1', handler: { proxy: { host: 'localhost', port: backendPort, timeout: 5 } } },
                         { method: 'GET', path: '/timeout2', handler: { proxy: { host: 'localhost', port: backendPort } } },
                         { method: 'GET', path: '/single', handler: { proxy: { mapUri: mapSingleUri } } },
@@ -505,22 +503,6 @@ describe('Proxy', function () {
         });
     });
 
-    it('errors on invalid response stream', function (done) {
-
-        var orig = Nipple.parse;
-        Nipple.parse = function (res, callback) {
-
-            Nipple.parse = orig;
-            callback(Hapi.error.internal('Fake error'));
-        };
-
-        server.inject('/custom', function (res) {
-
-            expect(res.statusCode).to.equal(500);
-            done();
-        });
-    });
-
     it('errors when proxied request times out', function (done) {
 
         server.inject('/timeout1', function (res) {