[#317] allow authors to provide custom error handler (including skip error handling) (#332)

Author: monkpow

README.md

@@ -250,6 +250,38 @@ app.use('/proxy', proxy('www.google.com', {
 }));
 ```
 
+### proxyErrorHandler
+
+By default, ```express-http-proxy``` will pass any errors except ECONNRESET to
+next, so that your application can handle or react to them, or just drop
+through to your default error handling.   ECONNRESET errors are immediately
+returned to the user for historical reasons.
+
+If you would like to modify this behavior, you can provide your own ```proxyErrorHandler```.
+
+
+```js
+// Example of skipping all error handling.
+
+app.use(proxy('localhost:12346', {
+  proxyErrorHandler: function(err, res, next) {
+    next(err);
+  }
+}));
+
+
+// Example of rolling your own
+
+app.use(proxy('localhost:12346', {
+  proxyErrorHandler: function(err, res, next) {
+    switch (err && err.code) {
+      case 'ECONNRESET':    { return res.status(405).send('504 became 405'); }
+      case 'ECONNREFUSED':  { return res.status(200).send('gotcher back'); }
+      default:              { next(err); }
+    }
+}}));
+```
+
 
 
 #### proxyReqOptDecorator  (supports Promise form)

app/steps/decorateProxyErrors.js renamed to app/steps/handleProxyErrors.js

@@ -5,15 +5,15 @@ var debug = require('debug')('express-http-proxy');
 function connectionResetHandler(err, res) {
   if (err && err.code === 'ECONNRESET') {
     debug('Error: Connection reset due to timeout.');
-    res.setHeader('X-Timout-Reason', 'express-http-proxy reset the request.');
+    res.setHeader('X-Timeout-Reason', 'express-http-proxy reset the request.');
     res.writeHead(504, {'Content-Type': 'text/plain'});
     res.end();
   }
 }
 
 function handleProxyErrors(err, res, next) {
   switch (err && err.code) {
-    case 'ECONNRESET':  { return connectionResetHandler(err, res); }
+    case 'ECONNRESET':  { return connectionResetHandler(err, res, next); }
     default:            { next(err); }
   }
 }

index.js

@@ -15,7 +15,7 @@ var decorateProxyReqBody         = require('./app/steps/decorateProxyReqBody');
 var decorateProxyReqOpts         = require('./app/steps/decorateProxyReqOpts');
 var decorateUserRes              = require('./app/steps/decorateUserRes');
 var decorateUserResHeaders       = require('./app/steps/decorateUserResHeaders');
-var decorateProxyErrors          = require('./app/steps/decorateProxyErrors');
+var handleProxyErrors          = require('./app/steps/handleProxyErrors');
 var maybeSkipToNextHandler       = require('./app/steps/maybeSkipToNextHandler');
 var prepareProxyReq              = require('./app/steps/prepareProxyReq');
 var resolveProxyHost             = require('./app/steps/resolveProxyHost');
@@ -47,7 +47,10 @@ module.exports = function proxy(host, userOptions) {
       .then(decorateUserRes)
       .then(sendUserRes)
       .catch(function(err) {
-        decorateProxyErrors(err, res, next);
+        var resolver = (container.options.proxyErrorHandler) ?
+          container.options.proxyErrorHandler :
+          handleProxyErrors;
+        resolver(err, res, next);
       });
   };
 };

lib/resolveOptions.js

@@ -38,6 +38,7 @@ function resolveOptions(options) {
     proxyReqBodyDecorator: options.proxyReqBodyDecorator,
     userResDecorator: options.userResDecorator || options.intercept,
     userResHeaderDecorator: options.userResHeaderDecorator,
+    proxyErrorHandler: options.proxyErrorHandler,
     filter: options.filter || defaultFilter,
     // For backwards compatability, we default to legacy behavior for newly added settings.
     parseReqBody: isUnset(options.parseReqBody) ? true : options.parseReqBody,

test/handleProxyError.js

@@ -0,0 +1,137 @@
+'use strict';
+
+var assert = require('assert');
+var express = require('express');
+var request = require('supertest');
+var proxy = require('../');
+var proxyTarget = require('../test/support/proxyTarget');
+var proxyRouteFn = [{
+  method: 'get',
+  path: '/:errorCode',
+  fn: function(req, res) {
+    var errorCode = req.params.errorCode;
+    if (errorCode === 'timeout') {
+      return res.status(504).send('mock timeout');
+    }
+    return res.status(parseInt(errorCode)).send('test case error');
+  }
+}];
+
+describe('error handling can be over-ridden by user', function() {
+  var app = express();
+  var proxyServer;
+
+  beforeEach(function() {
+    proxyServer = proxyTarget(12346, 100, proxyRouteFn);
+    app = express();
+  });
+
+  afterEach(function() {
+    proxyServer.close();
+  });
+
+  describe('when user provides a null function', function() {
+
+    describe('when author sets a timeout that fires', function() {
+      it('passes 504 directly to client', function(done) {
+        app.use(proxy('localhost:12346', {
+          timeout: 1,
+        }));
+
+        request(app)
+          .get('/200')
+          .expect(504)
+          .expect('X-Timeout-Reason', 'express-http-proxy reset the request.')
+          .end(done);
+      });
+    });
+
+    it('passes status code (e.g. 504) directly to the client', function(done) {
+      app.use(proxy('localhost:12346'));
+      request(app)
+        .get('/504')
+        .expect(504)
+        .expect(function(res) {
+          assert(res.text === 'test case error');
+          return res;
+        })
+        .end(done);
+    });
+
+    it('passes status code (e.g. 500) back to the client', function(done) {
+      app.use(proxy('localhost:12346'));
+      request(app)
+        .get('/500')
+        .expect(500)
+        .end(function(err, res) {
+          assert(res.text === 'test case error');
+          done();
+        });
+    });
+  });
+
+  describe('when user provides a handler function', function() {
+    var intentionallyWeirdStatusCode = 399;
+    var intentionallyQuirkyStatusMessage = 'monkey skunky';
+
+    describe('when author sets a timeout that fires', function() {
+      it('allows author to skip handling and handle in application step', function(done) {
+        app.use(proxy('localhost:12346', {
+          timeout: 1,
+          proxyErrorHandler: function(err, res, next) {
+            next(err);
+          }
+        }));
+
+        app.use(function(err, req, res, next) { // jshint ignore:line
+          if (err.code === 'ECONNRESET') {
+            res.status(intentionallyWeirdStatusCode).send(intentionallyQuirkyStatusMessage);
+          }
+        });
+
+        request(app)
+          .get('/200')
+          .expect(function(res) {
+            assert(res.text === intentionallyQuirkyStatusMessage);
+            return res;
+          })
+          .expect(intentionallyWeirdStatusCode)
+          .end(done);
+      });
+    });
+
+    it('allows authors to sub in their own handling', function(done) {
+      app.use(proxy('localhost:12346', {
+        timeout: 1,
+        proxyErrorHandler: function(err, res, next) {
+          console.log('bet its here');
+          switch (err && err.code) {
+            case 'ECONNRESET':    { return res.status(405).send('504 became 405'); }
+            case 'ECONNREFUSED':  { return res.status(200).send('gotcher back'); }
+            default:              { next(err); }
+          }
+      }}));
+
+      request(app)
+        .get('/timeout')
+        .expect(405)
+        .expect(function(res) {
+          assert(res.text === '504 became 405');
+          return res;
+        })
+        .end(done);
+    });
+
+    it('passes status code (e.g. 500) back to the client', function(done) {
+      app.use(proxy('localhost:12346'));
+      request(app)
+        .get('/500')
+        .expect(500)
+        .end(function(err, res) {
+          assert(res.text === 'test case error');
+          done();
+        });
+    });
+  });
+
+});

test/timeout.js

@@ -31,7 +31,7 @@ describe('honors timeout option', function() {
     request(http)
       .get('/')
       .expect(504)
-      .expect('X-Timout-Reason', 'express-http-proxy reset the request.')
+      .expect('X-Timeout-Reason', 'express-http-proxy reset the request.')
       .end(done);
   }