Repository: dherault/serverless-offline (js)

Issue:
Generic error when using the latest version
I have been using serverless-offline for many months on my current project. My AWS setup has a react frontend with https enabled. Everything still worked today, but then I did a full re-build by deleting node_modules. Suddenly I was getting this error on the command line:
```
sessionID: SIJ0soLd7YGQdew5I3h5BHnzlyOVIccd
Debug: internal, implementation, error 
    TypeError: Uncaught error: Cannot read property 'done' of undefined
    at createLambdaContext (/home/max/Documents/code/ark/trinity/nextjs/node_modules/serverless-offline/src/index.js:671:44)
    at process.nextTick (/home/max/Documents/code/ark/trinity/nextjs/node_modules/serverless-http/lib/dispatch.js:7:9)
    at _combinedTickCallback (internal/process/next_tick.js:132:7)
    at process._tickDomainCallback (internal/process/next_tick.js:219:9)
```

Looking at the source code at that line, it seems like maybe the request is empty. The error is a generic error about promises, I think. After some trial and error I found that downgrading the version of serverless-offline fixes the problem.

My old package.json:
```
"serverless-offline": "^4.0.0",
```
Downgrade to 4.10.0 and it will work again:
```
"serverless-offline": "=4.10.0",
```

<details> 
  <summary>Full logs from building to the error:</summary>
   <div>

(09:11:57)[max@think nextjs]$ yarn; node-prune; yarn build; yarn devel
yarn install v1.12.3
[1/4] Resolving packages...
[2/4] Fetching packages...
info fsevents@1.2.9: The platform "linux" is incompatible with this module.
info "fsevents@1.2.9" is an optional dependency and failed compatibility check. Excluding it from installation.
[3/4] Linking dependencies...
warning "@material-ui/styles > jss-plugin-vendor-prefixer@10.0.0-alpha.7" has incorrect peer dependency "jss@10.0.0-alpha.3".
warning "@zeit/next-css > css-loader@1.0.0" has unmet peer dependency "webpack@^4.0.0".
warning "@zeit/next-css > mini-css-extract-plugin@0.4.3" has unmet peer dependency "webpack@^4.4.0".
warning "@zeit/next-sass > sass-loader@6.0.6" has unmet peer dependency "webpack@^2.0.0 || >= 3.0.0-rc.0 || ^3.0.0".
warning "next-images > file-loader@3.0.1" has unmet peer dependency "webpack@^4.0.0".
warning "next-images > url-loader@1.1.2" has unmet peer dependency "webpack@^3.0.0 || ^4.0.0".
warning " > react-dropzone@10.1.4" has incorrect peer dependency "react@>= 16.8".
[4/4] Building fresh packages...
success Saved lockfile.
Done in 9.39s.

         files total 40,930
       files removed 12,174
        size removed 63 MB
            duration 326ms

yarn run v1.12.3
$ rm -rf .next && next build
Creating an optimized production build ...

> Using external babel configuration
> Location: "/home/max/Documents/code/ark/trinity/nextjs/.babelrc"
Compiled successfully.

 ┌ /
 ├ /_app
 ├ /_document
 ├ /_error
 ├ /about
 ├ /additem
 ├ /archive
 ├ /cart
 ├ /help
 ├ /history
 ├ /item
 ├ /login
 ├ /manage
 ├ /profile
 └ /shop

Done in 52.04s.
yarn run v1.12.3
$ yarn prod --stage development
$ NODE_ENV=production yarn offline --port=3000 --host=0.0.0.0 --stage development
$ sls offline start --port=3000 --host=0.0.0.0 --stage development
Serverless: Starting Offline: development/us-east-1.

Serverless: Routes for imageWatermark:
Serverless: (none)

Serverless: Routes for getCounts:
Serverless: POST /counts

Serverless: Routes for getCartGuest:
Serverless: POST /get_cart_guest

Serverless: Routes for getCartAuth:
Serverless: POST /get_cart_auth
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for checkUserDb:
Serverless: POST /checkUserDb
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for imageCleanup:
Serverless: (none)

Serverless: Routes for addItem:
Serverless: POST /additem
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for emailOrderStatus:
Serverless: (none)

Serverless: Routes for sendEmail:
Serverless: (none)

Serverless: Routes for deleteItem:
Serverless: DELETE /item
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for saveOrderStatus:
Serverless: POST /save_order_status
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for getUser:
Serverless: POST /get_user
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for pastOrders:
Serverless: POST /past_orders
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for allItems:
Serverless: POST /all_records
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for restore:
Serverless: POST /restore
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for itemQuery:
Serverless: POST /query

Serverless: Routes for itemLoad:
Serverless: POST /item_data
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for itemSave:
Serverless: POST /item
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for cartGuest:
Serverless: POST /cart_guest

Serverless: Routes for cartAuth:
Serverless: POST /cart_auth
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for buyGuest:
Serverless: POST /buy_guest

Serverless: Routes for buyAuth:
Serverless: POST /buy_auth
Serverless: WARNING: Serverless Offline does not support the AWS_IAM authorization type

Serverless: Routes for react:
Serverless: ANY /
Serverless: ANY /{proxy*}

Serverless: Offline listening on http://0.0.0.0:3000

Serverless: ANY / (λ: react)
sessionID: SIJ0soLd7YGQdew5I3h5BHnzlyOVIccd
Debug: internal, implementation, error 
    TypeError: Uncaught error: Cannot read property 'done' of undefined
    at createLambdaContext (/home/max/Documents/code/ark/trinity/nextjs/node_modules/serverless-offline/src/index.js:671:44)
    at process.nextTick (/home/max/Documents/code/ark/trinity/nextjs/node_modules/serverless-http/lib/dispatch.js:7:9)
    at _combinedTickCallback (internal/process/next_tick.js:132:7)
    at process._tickDomainCallback (internal/process/next_tick.js:219:9)

</div>
</details>

Pull request description:
Cleanup on a promise continuation when handler is async to fix #659
It looks like when the lambda handler is async and defers execution, the function scheduled in the setTimeout call would clean up the requests object.  The problem is that the handler is still in flight and when it resumes execution, the request object is gone.

This fix keeps the original behavior when the handler is not created or is not a promise, but will try clean up on a promise continuation if it is a promise.  I'd like to use `.finally(cleanup)` but that isn't supported in node 8, so that's why I went with `.then(cleanup, cleanup)`.  It looks like we don't care about the resolved value, so I think that should work fine.

Patch:
diff --git a/src/index.js b/src/index.js
index 5d5b030e..71a67030 100644
--- a/src/index.js
+++ b/src/index.js
@@ -957,8 +957,10 @@ class Offline {
 
             // Finally we call the handler
             debugLog('_____ CALLING HANDLER _____');
+
+            let x;
             try {
-              const x = handler(event, lambdaContext, lambdaContext.done);
+              x = handler(event, lambdaContext, lambdaContext.done);
 
               // Promise support
               if (!this.requests[requestId].done) {
@@ -970,10 +972,13 @@ class Offline {
               return this._reply500(response, `Uncaught error in your '${funName}' handler`, error);
             }
             finally {
-              setTimeout(() => {
+              const cleanup = () => {
                 this._clearTimeout(requestId);
                 delete this.requests[requestId];
-              }, 0);
+              };
+
+              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 ea7e991a..be39ac34 100644
--- a/test/integration/offline.js
+++ b/test/integration/offline.js
@@ -546,6 +546,34 @@ describe('Offline', () => {
         done();
       });
     });
+
+    it('should support handler returning Promise that defers', done => {
+      const offline = new OfflineBuilder(new ServerlessBuilder(serverless))
+        .addFunctionHTTP('index', {
+          path: 'index',
+          method: 'GET',
+        }, () => 
+          new Promise(resolve => 
+            setTimeout(() => 
+              resolve({
+                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();
+      });
+    });
   });
 
   context('with HEAD support', () => {