Repository: dherault/serverless-offline
Pull request #665 (2019-05-20 03:02:00)

Call cleanup in callback and error handler instead of finally block.
This should address the second part of #659 where cleanup wasn't getting called properly for callback based requests that deferred execution.

I'm not sure what the original intent of the cleanup method was and this feels a bit like whack-a-mole. That said, all tests pass (including two new ones) and the test application linked in the issue works correctly. Speaking of tests, I have one test around error handling and I'm not sure how to suppress logging of the error to the screen while running the tests. Not sure if that matters, but it doesn't look nice.


Patch:
diff --git a/package.json b/package.json
index 9b78301e..5bb7e9f1 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "serverless-offline",
-  "version": "4.10.3",
+  "version": "4.10.2",
   "description": "Emulate AWS λ and API Gateway locally when developing your Serverless project",
   "license": "MIT",
   "main": "src/index.js",
diff --git a/src/index.js b/src/index.js
index 71a67030..35f16ca3 100644
--- a/src/index.js
+++ b/src/index.js
@@ -958,28 +958,30 @@ class Offline {
             // Finally we call the handler
             debugLog('_____ CALLING HANDLER _____');
 
+            const cleanup = () => {
+              this._clearTimeout(requestId);
+              delete this.requests[requestId];
+            };
+            
             let x;
             try {
-              x = handler(event, lambdaContext, lambdaContext.done);
+              x = handler(event, lambdaContext, (err, result) => {
+                setTimeout(cleanup, 0);
+                
+                return lambdaContext.done(err, result);
+              });
 
               // Promise support
               if (!this.requests[requestId].done) {
-                if (x && typeof x.then === 'function' && typeof x.catch === 'function') x.then(lambdaContext.succeed).catch(lambdaContext.fail);
+                if (x && typeof x.then === 'function' && typeof x.catch === 'function') x.then(lambdaContext.succeed).catch(lambdaContext.fail).then(cleanup, cleanup);
                 else if (x instanceof Error) lambdaContext.fail(x);
               }
             }
             catch (error) {
+              cleanup();
+              
               return this._reply500(response, `Uncaught error in your '${funName}' handler`, error);
             }
-            finally {
-              const cleanup = () => {
-                this._clearTimeout(requestId);
-                delete this.requests[requestId];
-              };
-
-              if (x && typeof x.then === 'function' && typeof x.catch === 'function') x.then(cleanup, cleanup);
-              else setTimeout(cleanup, 0);
-            }
           },
         });
       });


Test patch:
diff --git a/test/integration/offline.js b/test/integration/offline.js
index be39ac34..a4aa77d8 100644
--- a/test/integration/offline.js
+++ b/test/integration/offline.js
@@ -574,6 +574,54 @@ describe('Offline', () => {
         done();
       });
     });
+
+    it('should support handler that defers and uses done()', done => {
+      const offline = new OfflineBuilder(new ServerlessBuilder(serverless))
+        .addFunctionHTTP('index', {
+          path: 'index',
+          method: 'GET',
+        }, (request, context, cb) => 
+          setTimeout(() => 
+            cb(null, {
+              statusCode: 200,
+              body: JSON.stringify({ message: 'Hello World' }),
+            }), 
+          10)
+        ).toObject();
+
+      offline.inject({
+        method: 'GET',
+        url: '/index',
+        payload: { data: 'input' },
+      }, res => {
+        expect(res.headers).to.have.property('content-type').which.contains('application/json');
+        expect(res.statusCode).to.eq(200);
+        expect(res.payload).to.eq('{"message":"Hello World"}');
+        done();
+      });
+    });
+
+    it('should support handler that throws and uses done()', done => {
+      const offline = new OfflineBuilder(new ServerlessBuilder(serverless))
+        .addFunctionHTTP('index', {
+          path: 'index',
+          method: 'GET',
+        }, () => {
+          throw new Error('This is an error');
+        }
+        ).toObject();
+
+      offline.inject({
+        method: 'GET',
+        url: '/index',
+        payload: { data: 'input' },
+      }, res => {
+        expect(res.headers).to.have.property('content-type').which.contains('application/json');
+        expect(res.statusCode).to.eq(200);
+        done();
+      });
+    });
+
   });
 
   context('with HEAD support', () => {