From 1427e4513e8802162a8674c94108f3d9871dc561 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 17 May 2017 00:57:17 -0700 Subject: [PATCH 1/3] error-reporting: Address unresponsive Hapi apps The Hapi plugin was too restrictive when determining if it should continue a reply, which could cause some Hapi apps to become unresponsive. --- packages/error-reporting/src/interfaces/hapi.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/error-reporting/src/interfaces/hapi.js b/packages/error-reporting/src/interfaces/hapi.js index 9f8855a2853..35ee6eeaca7 100644 --- a/packages/error-reporting/src/interfaces/hapi.js +++ b/packages/error-reporting/src/interfaces/hapi.js @@ -92,7 +92,7 @@ function makeHapiPlugin(client, config) { client.sendError(em); } - if (isObject(reply) && isFunction(reply.continue)) { + if (reply && isFunction(reply.continue)) { reply.continue(); } }); From 92ec62ad4d83f6c243a61ac1ec6cfc40e481de57 Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 17 May 2017 10:38:07 -0700 Subject: [PATCH 2/3] Add a test case --- .../test/unit/interfaces/hapi.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/error-reporting/test/unit/interfaces/hapi.js b/packages/error-reporting/test/unit/interfaces/hapi.js index 0ed136db10b..5db46dafb6b 100644 --- a/packages/error-reporting/test/unit/interfaces/hapi.js +++ b/packages/error-reporting/test/unit/interfaces/hapi.js @@ -113,7 +113,8 @@ describe('Hapi interface', function() { afterEach(function() { fakeServer.removeAllListeners(); }); - it('Should call continue when a boom is emitted', function(done) { + it('Should call continue when a boom is emitted if reply is an object', + function(done) { plugin.register(fakeServer, null, function() {}); fakeServer.emit(EVENT, {response: {isBoom: true}}, { @@ -124,6 +125,21 @@ describe('Hapi interface', function() { } ); }); + it('Should call continue when a boom is emitted if reply is a function', + function(done) { + // Manually testing has shown that in actual usage the `reply` object + // provided to listeners of the `onPreResponse` event can be a function + // that has a `continue` property that is a function. + // If `reply.continue()` is not invoked in this situation, the Hapi + // app will become unresponsive. + plugin.register(fakeServer, null, function() {}); + var reply = function() {}; + reply.continue = function() { + // The continue function should be called + done(); + }; + fakeServer.emit(EVENT, {response: {isBoom: true}}, reply); + }); it('Should call sendError when a boom is received', function(done) { var fakeClient = { sendError: function(err) { From 8b43f0a643725abfdb19c43159a04dc5e02c69cb Mon Sep 17 00:00:00 2001 From: Dominic Kramer Date: Wed, 17 May 2017 13:25:22 -0700 Subject: [PATCH 3/3] Fix indentation --- .../test/unit/interfaces/hapi.js | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/packages/error-reporting/test/unit/interfaces/hapi.js b/packages/error-reporting/test/unit/interfaces/hapi.js index 5db46dafb6b..ae2c491d215 100644 --- a/packages/error-reporting/test/unit/interfaces/hapi.js +++ b/packages/error-reporting/test/unit/interfaces/hapi.js @@ -114,31 +114,31 @@ describe('Hapi interface', function() { fakeServer.removeAllListeners(); }); it('Should call continue when a boom is emitted if reply is an object', - function(done) { - plugin.register(fakeServer, null, function() {}); - fakeServer.emit(EVENT, {response: {isBoom: true}}, - { - continue: function() { - // The continue function should be called - done(); + function(done) { + plugin.register(fakeServer, null, function() {}); + fakeServer.emit(EVENT, {response: {isBoom: true}}, + { + continue: function() { + // The continue function should be called + done(); + } } - } - ); + ); }); it('Should call continue when a boom is emitted if reply is a function', - function(done) { - // Manually testing has shown that in actual usage the `reply` object - // provided to listeners of the `onPreResponse` event can be a function - // that has a `continue` property that is a function. - // If `reply.continue()` is not invoked in this situation, the Hapi - // app will become unresponsive. - plugin.register(fakeServer, null, function() {}); - var reply = function() {}; - reply.continue = function() { - // The continue function should be called - done(); - }; - fakeServer.emit(EVENT, {response: {isBoom: true}}, reply); + function(done) { + // Manually testing has shown that in actual usage the `reply` object + // provided to listeners of the `onPreResponse` event can be a function + // that has a `continue` property that is a function. + // If `reply.continue()` is not invoked in this situation, the Hapi + // app will become unresponsive. + plugin.register(fakeServer, null, function() {}); + var reply = function() {}; + reply.continue = function() { + // The continue function should be called + done(); + }; + fakeServer.emit(EVENT, {response: {isBoom: true}}, reply); }); it('Should call sendError when a boom is received', function(done) { var fakeClient = {