Merge pull request #4100 from weseek/imprv/slackbot-error-handling-arch

Imprv/slackbot error handling arch

src/server/models/vo/slackbot-error.js

@@ -0,0 +1,22 @@
+/**
+ * Error class for slackbot service
+ */
+class SlackbotError extends Error {
+
+  constructor({
+    method, to, popupMessage, mainMessage,
+  } = {}) {
+    super();
+    this.method = method;
+    this.to = to;
+    this.popupMessage = popupMessage;
+    this.mainMessage = mainMessage;
+  }
+
+  static isSlackbotError(obj) {
+    return obj instanceof this;
+  }
+
+}
+
+module.exports = SlackbotError;

src/server/routes/apiv3/slack-integration.js

@@ -9,6 +9,7 @@ const { verifySlackRequest, generateWebClient } = require('@growi/slack');
 const logger = loggerFactory('growi:routes:apiv3:slack-integration');
 const router = express.Router();
 const SlackAppIntegration = mongoose.model('SlackAppIntegration');
+const { respondIfSlackbotError } = require('../../service/slack-command-handler/respond-if-slackbot-error');
 
 module.exports = (crowi) => {
   this.app = crowi.express;
@@ -106,9 +107,10 @@ module.exports = (crowi) => {
     try {
       await crowi.slackBotService.handleCommandRequest(command, client, body, args);
     }
-    catch (error) {
-      logger.error(error);
+    catch (err) {
+      await respondIfSlackbotError(client, body, err);
     }
+
   }
 
   router.post('/commands', addSigningSecretToReq, verifySlackRequest, async(req, res) => {
@@ -151,10 +153,20 @@ module.exports = (crowi) => {
     try {
       switch (type) {
         case 'block_actions':
-          await crowi.slackBotService.handleBlockActionsRequest(client, payload);
+          try {
+            await crowi.slackBotService.handleBlockActionsRequest(client, payload);
+          }
+          catch (err) {
+            await respondIfSlackbotError(client, req.body, err);
+          }
           break;
         case 'view_submission':
-          await crowi.slackBotService.handleViewSubmissionRequest(client, payload);
+          try {
+            await crowi.slackBotService.handleViewSubmissionRequest(client, payload);
+          }
+          catch (err) {
+            await respondIfSlackbotError(client, req.body, err);
+          }
           break;
         default:
           break;

src/server/service/slack-command-handler/create-page-service.js

@@ -1,8 +1,8 @@
-const { markdownSectionBlock } = require('@growi/slack');
 const logger = require('@alias/logger')('growi:service:CreatePageService');
 const { reshapeContentsBody } = require('@growi/slack');
 const mongoose = require('mongoose');
 const pathUtils = require('growi-commons').pathUtils;
+const SlackbotError = require('../../models/vo/slackbot-error');
 
 class CreatePageService {
 
@@ -31,13 +31,13 @@ class CreatePageService {
       });
     }
     catch (err) {
-      client.chat.postMessage({
-        channel: payload.user.id,
-        blocks: [
-          markdownSectionBlock(`Cannot create new page to existed path\n *Contents* :memo:\n ${reshapedContentsBody}`)],
+      logger.error('Failed to create page in GROWI.', err);
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'Cannot create new page to existed path.',
+        mainMessage: `Cannot create new page to existed path\n *Contents* :memo:\n ${reshapedContentsBody}`,
       });
-      logger.error('Failed to create page in GROWI.');
-      throw err;
     }
   }
 

src/server/service/slack-command-handler/create.js

@@ -1,5 +1,4 @@
 const { markdownSectionBlock, inputSectionBlock } = require('@growi/slack');
-const logger = require('@alias/logger')('growi:service:SlackCommandHandler:create');
 
 module.exports = (crowi) => {
   const CreatePageService = require('./create-page-service');
@@ -8,46 +7,32 @@ module.exports = (crowi) => {
   const handler = new BaseSlackCommandHandler();
 
   handler.handleCommand = async(client, body) => {
-    try {
-      await client.views.open({
-        trigger_id: body.trigger_id,
+    await client.views.open({
+      trigger_id: body.trigger_id,
 
-        view: {
-          type: 'modal',
-          callback_id: 'create:createPage',
-          title: {
-            type: 'plain_text',
-            text: 'Create Page',
-          },
-          submit: {
-            type: 'plain_text',
-            text: 'Submit',
-          },
-          close: {
-            type: 'plain_text',
-            text: 'Cancel',
-          },
-          blocks: [
-            markdownSectionBlock('Create new page.'),
-            inputSectionBlock('path', 'Path', 'path_input', false, '/path'),
-            inputSectionBlock('contents', 'Contents', 'contents_input', true, 'Input with Markdown...'),
-          ],
-          private_metadata: JSON.stringify({ channelId: body.channel_id }),
+      view: {
+        type: 'modal',
+        callback_id: 'create:createPage',
+        title: {
+          type: 'plain_text',
+          text: 'Create Page',
+        },
+        submit: {
+          type: 'plain_text',
+          text: 'Submit',
+        },
+        close: {
+          type: 'plain_text',
+          text: 'Cancel',
         },
-      });
-    }
-    catch (err) {
-      logger.error('Failed to create a page.');
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Failed To Create',
         blocks: [
-          markdownSectionBlock(`*Failed to create new page.*\n ${err}`),
+          markdownSectionBlock('Create new page.'),
+          inputSectionBlock('path', 'Path', 'path_input', false, '/path'),
+          inputSectionBlock('contents', 'Contents', 'contents_input', true, 'Input with Markdown...'),
         ],
-      });
-      throw err;
-    }
+        private_metadata: JSON.stringify({ channelId: body.channel_id }),
+      },
+    });
   };
 
   handler.handleBlockActions = async function(client, payload, handlerMethodName) {

src/server/service/slack-command-handler/respond-if-slackbot-error.js

@@ -0,0 +1,64 @@
+const logger = require('@alias/logger')('growi:service:SlackCommandHandler:slack-bot-response');
+const { markdownSectionBlock } = require('@growi/slack');
+const SlackbotError = require('../../models/vo/slackbot-error');
+
+async function respondIfSlackbotError(client, body, err) {
+  // check if the request is to /commands OR /interactions
+  const isInteraction = !body.channel_id;
+
+  // throw non-SlackbotError
+  if (!SlackbotError.isSlackbotError(err)) {
+    logger.error(`A non-SlackbotError error occured.\n${err.toString()}`);
+    throw err;
+  }
+
+  // for both postMessage and postEphemeral
+  let toChannel;
+  // for only postEphemeral
+  let toUser;
+  // decide which channel to send to
+  switch (err.to) {
+    case 'dm':
+      toChannel = isInteraction ? JSON.parse(body.payload).user.id : body.user_id;
+      toUser = toChannel;
+      break;
+    case 'channel':
+      toChannel = isInteraction ? JSON.parse(body.payload).channel.id : body.channel_id;
+      toUser = isInteraction ? JSON.parse(body.payload).user.id : body.user_id;
+      break;
+    default:
+      logger.error('The "to" property of SlackbotError must be "dm" or "channel".');
+      break;
+  }
+
+  // argumentObj object to pass to postMessage OR postEphemeral
+  let argumentsObj = {};
+  switch (err.method) {
+    case 'postMessage':
+      argumentsObj = {
+        channel: toChannel,
+        text: err.popupMessage,
+        blocks: [
+          markdownSectionBlock(err.mainMessage),
+        ],
+      };
+      break;
+    case 'postEphemeral':
+      argumentsObj = {
+        channel: toChannel,
+        user: toUser,
+        text: err.popupMessage,
+        blocks: [
+          markdownSectionBlock(err.mainMessage),
+        ],
+      };
+      break;
+    default:
+      logger.error('The "method" property of SlackbotError must be "postMessage" or "postEphemeral".');
+      break;
+  }
+
+  await client.chat[err.method](argumentsObj);
+}
+
+module.exports = { respondIfSlackbotError };

src/server/service/slack-command-handler/search.js

@@ -3,6 +3,7 @@ const logger = require('@alias/logger')('growi:service:SlackCommandHandler:searc
 const { markdownSectionBlock, divider } = require('@growi/slack');
 const { formatDistanceStrict } = require('date-fns');
 const axios = require('axios');
+const SlackbotError = require('../../models/vo/slackbot-error');
 
 const PAGINGLIMIT = 10;
 
@@ -17,15 +18,12 @@ module.exports = (crowi) => {
     }
     catch (err) {
       logger.error('Failed to get search results.', err);
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Failed To Search',
-        blocks: [
-          markdownSectionBlock('*Failed to search.*\n Hint\n `/growi search [keyword]`'),
-        ],
+      throw new SlackbotError({
+        method: 'postEphemeral',
+        to: 'channel',
+        popupMessage: 'Failed To Search',
+        mainMessage: '*Failed to search.*\n Hint\n `/growi search [keyword]`',
       });
-      throw new Error('/growi command:search: Failed to search');
     }
 
     const appUrl = crowi.appService.getSiteUrl();
@@ -140,26 +138,12 @@ module.exports = (crowi) => {
     }
     blocks.push(actionBlocks);
 
-    try {
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Successed To Search',
-        blocks,
-      });
-    }
-    catch (err) {
-      logger.error('Failed to post ephemeral message.', err);
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Failed to post ephemeral message.',
-        blocks: [
-          markdownSectionBlock(err.toString()),
-        ],
-      });
-      throw new Error(err.message);
-    }
+    await client.chat.postEphemeral({
+      channel: body.channel_id,
+      user: body.user_id,
+      text: 'Successed To Search',
+      blocks,
+    });
   };
 
   handler.handleBlockActions = async function(client, payload, handlerMethodName) {
@@ -210,15 +194,12 @@ module.exports = (crowi) => {
     }
     catch (err) {
       logger.error('Failed to get search results.', err);
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Failed To Search',
-        blocks: [
-          markdownSectionBlock('*Failed to search.*\n Hint\n `/growi search [keyword]`'),
-        ],
+      throw new SlackbotError({
+        method: 'postEphemeral',
+        to: 'channel',
+        popupMessage: 'Failed To Search',
+        mainMessage: '*Failed to search.*\n Hint\n `/growi search [keyword]`',
       });
-      throw new Error('/growi command:search: Failed to search');
     }
 
     const appUrl = crowi.appService.getSiteUrl();
@@ -333,26 +314,12 @@ module.exports = (crowi) => {
     }
     blocks.push(actionBlocks);
 
-    try {
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Successed To Search',
-        blocks,
-      });
-    }
-    catch (err) {
-      logger.error('Failed to post ephemeral message.', err);
-      await client.chat.postEphemeral({
-        channel: body.channel_id,
-        user: body.user_id,
-        text: 'Failed to post ephemeral message.',
-        blocks: [
-          markdownSectionBlock(err.toString()),
-        ],
-      });
-      throw new Error(err.message);
-    }
+    await client.chat.postEphemeral({
+      channel: body.channel_id,
+      user: body.user_id,
+      text: 'Successed To Search',
+      blocks,
+    });
   };
 
   handler.dismissSearchResults = async function(client, payload) {

src/server/service/slack-command-handler/togetter.js

@@ -4,6 +4,7 @@ const {
 const { parse, format } = require('date-fns');
 const axios = require('axios');
 const logger = require('@alias/logger')('growi:service:SlackBotService:togetter');
+const SlackbotError = require('../../models/vo/slackbot-error');
 
 module.exports = (crowi) => {
   const CreatePageService = require('./create-page-service');
@@ -54,16 +55,8 @@ module.exports = (crowi) => {
       await this.togetterCreatePageAndSendPreview(client, payload, path, channel, contentsBody);
     }
     catch (err) {
-      logger.error(err);
-      // upcoming GW-6853 will change: just throw Error() here and handle in slackbot.js
-      await client.chat.postMessage({
-        channel: payload.user.id,
-        text: err.message,
-        blocks: [
-          markdownSectionBlock(err.message),
-        ],
-      });
-      return;
+      logger.error('Error occured by togetter.');
+      throw err;
     }
   };
 
@@ -75,7 +68,12 @@ module.exports = (crowi) => {
     oldest = oldest.trim();
     latest = latest.trim();
     if (!path) {
-      throw new Error('Page path is required.');
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'Page path is required.',
+        mainMessage: 'Page path is required.',
+      });
     }
     /**
      * RegExp for datetime yyyy/MM/dd-HH:mm
@@ -84,17 +82,32 @@ module.exports = (crowi) => {
     const regexpDatetime = new RegExp(/^[12]\d\d\d\/(0[1-9]|1[012])\/(0[1-9]|[12][0-9]|3[01])-([01][0-9]|2[0123]):[0-5][0-9]$/);
 
     if (!regexpDatetime.test(oldest)) {
-      throw new Error('Datetime format for oldest must be yyyy/MM/dd-HH:mm');
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'Datetime format for oldest must be yyyy/MM/dd-HH:mm',
+        mainMessage: 'Datetime format for oldest must be yyyy/MM/dd-HH:mm',
+      });
     }
     if (!regexpDatetime.test(latest)) {
-      throw new Error('Datetime format for latest must be yyyy/MM/dd-HH:mm');
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'Datetime format for latest must be yyyy/MM/dd-HH:mm',
+        mainMessage: 'Datetime format for latest must be yyyy/MM/dd-HH:mm',
+      });
     }
     oldest = parse(oldest, 'yyyy/MM/dd-HH:mm', new Date()).getTime() / 1000 + grwTzoffset;
     // + 60s in order to include messages between hh:mm.00s and hh:mm.59s
     latest = parse(latest, 'yyyy/MM/dd-HH:mm', new Date()).getTime() / 1000 + grwTzoffset + 60;
 
     if (oldest > latest) {
-      throw new Error('Oldest datetime must be older than the latest date time.');
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'Oldest datetime must be older than the latest date time.',
+        mainMessage: 'Oldest datetime must be older than the latest date time.',
+      });
     }
 
     return { path, oldest, latest };
@@ -111,7 +124,12 @@ module.exports = (crowi) => {
 
     // return if no message found
     if (!result.messages.length) {
-      throw new Error('No message found from togetter command. Try again.');
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'No message found from togetter command. Try different datetime.',
+        mainMessage: 'No message found from togetter command. Try different datetime.',
+      });
     }
     return result;
   };
@@ -164,7 +182,13 @@ module.exports = (crowi) => {
       });
     }
     catch (err) {
-      throw new Error('Error occurred while creating a page.');
+      logger.error('Error occurred while creating a page.', err);
+      throw new SlackbotError({
+        method: 'postMessage',
+        to: 'dm',
+        popupMessage: 'Error occurred while creating a page.',
+        mainMessage: 'Error occurred while creating a page.',
+      });
     }
   };
 

src/server/service/slackbot.js

@@ -4,6 +4,7 @@ const { markdownSectionBlock } = require('@growi/slack');
 const S2sMessage = require('../models/vo/s2s-message');
 const S2sMessageHandlable = require('./s2s-messaging/handlable');
 
+
 class SlackBotService extends S2sMessageHandlable {
 
   constructor(crowi) {
@@ -91,7 +92,6 @@ class SlackBotService extends S2sMessageHandlable {
       await handler.handleBlockActions(client, payload, handlerMethodName);
     }
     catch (err) {
-      // response
       throw err;
     }
     return;
@@ -107,7 +107,6 @@ class SlackBotService extends S2sMessageHandlable {
       await handler.handleBlockActions(client, payload, handlerMethodName);
     }
     catch (err) {
-      // response
       throw err;
     }
     return;