Fix #1409 installerOptions.failure() options can be undefined (#1410)

* Fix #1409 installerOptions.failure() options can be undefined * Make the test code clearer
slackapi · Jan 12, 2022 · b30ae11 · b30ae11
1 parent df622ce
commit b30ae11
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 6 deletions.
diff --git a/packages/oauth/src/index.spec.js b/packages/oauth/src/index.spec.js
@@ -437,7 +437,7 @@ describe('OAuth', async () => {
         verifyStateParam: sinon.fake.resolves({})
       }
     });
-    it('should call the failure callback due to missing code query parameter on the URL', async () => {
+    it('should call the failure callback with a valid installOptions due to missing code query parameter on the URL', async () => {
       const req = { headers: { host: 'example.com'},  url: 'http://example.com' };
       let sent = false;
       const res = { send: () => { sent = true; } };
@@ -447,7 +447,10 @@ describe('OAuth', async () => {
           assert.fail('should have failed');
         },
         failure: async (error, installOptions, req, res) => {
-          assert.equal(error.code, ErrorCode.MissingCodeError)
+          // To detect future regressions, we verify if there is a valid installOptions here
+          // Refer to https://github.com/slackapi/node-slack-sdk/pull/1410 for the context
+          assert.isDefined(installOptions);
+          assert.equal(error.code, ErrorCode.MissingCodeError);
           res.send('failure');
         },
       }
@@ -466,6 +469,7 @@ describe('OAuth', async () => {
           assert.fail('should have failed');
         },
         failure: async (error, installOptions, req, res) => {
+          assert.isDefined(installOptions);
           assert.equal(error.code, ErrorCode.MissingStateError)
           res.send('failure');
         },
@@ -504,6 +508,7 @@ describe('OAuth', async () => {
           assert.fail('should have failed');
         },
         failure: async (error, installOptions, req, res) => {
+          assert.isDefined(installOptions);
           assert.equal(error.code, ErrorCode.AuthorizationError)
           res.send('failure');
         },
@@ -566,7 +571,6 @@ describe('OAuth', async () => {
         },
         failure: async (error, installOptions, req, res) => {
           assert.fail(error.message);
-          res.send('failure');
         },
       }
 
@@ -588,7 +592,6 @@ describe('OAuth', async () => {
         },
         failure: async (error, installOptions, req, res) => {
           assert.fail(error.message);
-          res.send('failure');
         },
       }
 

diff --git a/packages/oauth/src/index.ts b/packages/oauth/src/index.ts
@@ -515,13 +515,20 @@ export class InstallProvider {
     } catch (error: any) {
       this.logger.error(error);
 
+      if (!installOptions) {
+        // To make the `installOptions` type compatible with `CallbackOptions#failure` signature
+        const emptyInstallOptions: InstallURLOptions = { scopes: [] };
+        // eslint-disable-next-line no-param-reassign
+        installOptions = emptyInstallOptions;
+      }
+
       // Call the failure callback
       if (options !== undefined && options.failure !== undefined) {
         this.logger.debug('calling passed in options.failure');
-        options.failure(error, installOptions!, req, res);
+        options.failure(error, installOptions, req, res);
       } else {
         this.logger.debug('run built-in failure function');
-        callbackFailure(error, installOptions!, req, res);
+        callbackFailure(error, installOptions, req, res);
       }
     }
   }