fix(bot): bot name conflicts (#1098)

* fix(bot): add specific error message for bot name conflicts * chore(bot): add log for manually updating message endpoint * chore(bot): refine error design, name, reduce duplicated code
OfficeDev · May 19, 2021 · d9798cb · d9798cb
1 parent abe0efd
commit d9798cb
Show file tree

Hide file tree

Showing 8 changed files with 66 additions and 41 deletions.
diff --git a/packages/fx-core/src/plugins/resource/bot/appStudio/appStudio.ts b/packages/fx-core/src/plugins/resource/bot/appStudio/appStudio.ts
@@ -4,7 +4,7 @@ import { IAADDefinition } from "./interfaces/IAADDefinition";
 
 import { AxiosInstance, default as axios } from "axios";
 import {
-  CallAppStudioError,
+  AADAppCheckingError,
   ConfigUpdatingError,
   ProvisionError,
   SomethingMissingError,
@@ -96,7 +96,7 @@ export class AppStudio {
         axiosInstance.get(`${AppStudio.baseUrl}/api/aadapp/v2/${objectId}`)
       );
     } catch (e) {
-      throw new CallAppStudioError(LifecycleFuncNames.CHECK_AAD_APP, e);
+      throw new AADAppCheckingError(e);
     }
 
     if (!response || !response.data) {

diff --git a/packages/fx-core/src/plugins/resource/bot/azureOps.ts b/packages/fx-core/src/plugins/resource/bot/azureOps.ts
@@ -11,6 +11,7 @@ import {
   MessageEndpointUpdatingError,
   MissingSubscriptionRegistrationError,
   FreeServerFarmsQuotaError,
+  InvalidBotDataError,
 } from "./errors";
 import { CommonStrings, ConfigNames } from "./resources/strings";
 import * as utils from "./utils/common";
@@ -38,6 +39,8 @@ export class AzureOperations {
     } catch (e) {
       if (e.code === "MissingSubscriptionRegistration") {
         throw new MissingSubscriptionRegistrationError();
+      } else if (e.code === "InvalidBotData") {
+        throw new InvalidBotDataError(e);
       } else {
         throw new ProvisionError(CommonStrings.BOT_CHANNEL_REGISTRATION, e);
       }

diff --git a/packages/fx-core/src/plugins/resource/bot/constants.ts b/packages/fx-core/src/plugins/resource/bot/constants.ts
@@ -135,6 +135,7 @@ export class ErrorNames {
   public static readonly MISSING_SUBSCRIPTION_REGISTRATION_ERROR =
     "MissingSubscriptionRegistrationError";
   public static readonly FREE_SERVER_FARMS_QUOTA_ERROR = "FreeServerFarmsQuotaError";
+  public static readonly INVALID_BOT_DATA_ERROR = "InvalidBotDataError";
 }
 
 export class Links {

diff --git a/packages/fx-core/src/plugins/resource/bot/errors.ts b/packages/fx-core/src/plugins/resource/bot/errors.ts
@@ -42,7 +42,7 @@ export class PluginError extends Error {
 
 export class PreconditionError extends PluginError {
   constructor(message: string, suggestions: string[]) {
-    super(ErrorType.System, ErrorNames.PRECONDITION_ERROR, message, suggestions);
+    super(ErrorType.User, ErrorNames.PRECONDITION_ERROR, message, suggestions);
   }
 }
 
@@ -71,12 +71,12 @@ export class UserInputsError extends PluginError {
   }
 }
 
-export class CallAppStudioError extends PluginError {
-  constructor(apiName: string, innerError?: Error) {
+export class AADAppCheckingError extends PluginError {
+  constructor(innerError?: Error) {
     super(
       ErrorType.System,
       ErrorNames.CALL_APPSTUDIO_API_ERROR,
-      Messages.FailToCallAppStudio(apiName),
+      Messages.FailToCallAppStudioForCheckingAADApp,
       [Messages.RetryTheCurrentStep],
       innerError
     );
@@ -86,10 +86,10 @@ export class CallAppStudioError extends PluginError {
 export class ClientCreationError extends PluginError {
   constructor(clientName: string, innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.CLIENT_CREATION_ERROR,
       Messages.FailToCreateSomeClient(clientName),
-      [Messages.RetryTheCurrentStep],
+      [Messages.CheckOutputLogAndTryToFix, Messages.RetryTheCurrentStep],
       innerError
     );
   }
@@ -98,10 +98,10 @@ export class ClientCreationError extends PluginError {
 export class ProvisionError extends PluginError {
   constructor(resource: string, innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.PROVISION_ERROR,
       Messages.FailToProvisionSomeResource(resource),
-      [Messages.RetryTheCurrentStep],
+      [Messages.CheckOutputLogAndTryToFix, Messages.RetryTheCurrentStep],
       innerError
     );
   }
@@ -123,10 +123,10 @@ export class MissingSubscriptionRegistrationError extends PluginError {
 export class ConfigUpdatingError extends PluginError {
   constructor(configName: string, innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.CONFIG_UPDATING_ERROR,
       Messages.FailToUpdateConfigs(configName),
-      [Messages.RetryTheCurrentStep],
+      [Messages.CheckOutputLogAndTryToFix, Messages.RetryTheCurrentStep],
       innerError
     );
   }
@@ -157,10 +157,10 @@ export class PackDirExistenceError extends PluginError {
 export class ListPublishingCredentialsError extends PluginError {
   constructor(innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.LIST_PUBLISHING_CREDENTIALS_ERROR,
       Messages.FailToListPublishingCredentials,
-      [Messages.RetryTheCurrentStep],
+      [Messages.CheckOutputLogAndTryToFix, Messages.RetryTheCurrentStep],
       innerError
     );
   }
@@ -169,10 +169,10 @@ export class ListPublishingCredentialsError extends PluginError {
 export class ZipDeployError extends PluginError {
   constructor(innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.ZIP_DEPLOY_ERROR,
       Messages.FailToDoZipDeploy,
-      ["Please retry the deploy command."],
+      [Messages.CheckOutputLogAndTryToFix, Messages.RetryTheCurrentStep],
       innerError
     );
   }
@@ -181,10 +181,10 @@ export class ZipDeployError extends PluginError {
 export class MessageEndpointUpdatingError extends PluginError {
   constructor(endpoint: string, innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.MSG_ENDPOINT_UPDATING_ERROR,
       Messages.FailToUpdateMessageEndpoint(endpoint),
-      [Messages.RetryTheCurrentStep],
+      [Messages.CheckOutputLogAndTryToFix, Messages.RetryTheCurrentStep],
       innerError
     );
   }
@@ -193,7 +193,7 @@ export class MessageEndpointUpdatingError extends PluginError {
 export class DownloadError extends PluginError {
   constructor(url: string, innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.DOWNLOAD_ERROR,
       Messages.FailToDownloadFrom(url),
       ["Please check your network status and retry."],
@@ -205,7 +205,7 @@ export class DownloadError extends PluginError {
 export class TemplateProjectNotFoundError extends PluginError {
   constructor() {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.TEMPLATE_PROJECT_NOT_FOUND_ERROR,
       Messages.SomethingIsNotFound("Template project for scaffold"),
       [Messages.RetryTheCurrentStep]
@@ -214,11 +214,11 @@ export class TemplateProjectNotFoundError extends PluginError {
 }
 
 export class CommandExecutionError extends PluginError {
-  constructor(cmd: string, message: string, innerError?: Error) {
+  constructor(cmd: string, innerError?: Error) {
     super(
-      ErrorType.System,
+      ErrorType.User,
       ErrorNames.COMMAND_EXECUTION_ERROR,
-      Messages.CommandFailWithMessage(cmd, message),
+      Messages.CommandExecutionFailed(cmd),
       [Messages.CheckCommandOutputAndTryToFixIt, Messages.RetryTheCurrentStep],
       innerError
     );
@@ -237,3 +237,15 @@ export class FreeServerFarmsQuotaError extends PluginError {
     );
   }
 }
+
+export class InvalidBotDataError extends PluginError {
+  constructor(innerError: Error) {
+    super(
+      ErrorType.User,
+      ErrorNames.INVALID_BOT_DATA_ERROR,
+      innerError.message,
+      [Messages.DeleteExistingBotChannelRegistration, Messages.DeleteBotAfterAzureAccountSwitching],
+      innerError
+    );
+  }
+}
diff --git a/packages/fx-core/src/plugins/resource/bot/languageStrategy.ts b/packages/fx-core/src/plugins/resource/bot/languageStrategy.ts
@@ -94,11 +94,7 @@ export class LanguageStrategy {
         await utils.execute("npm install", packDir);
         await utils.execute("npm run build", packDir);
       } catch (e) {
-        throw new CommandExecutionError(
-          `${Commands.NPM_INSTALL},${Commands.NPM_BUILD}`,
-          e.message,
-          e
-        );
+        throw new CommandExecutionError(`${Commands.NPM_INSTALL},${Commands.NPM_BUILD}`, e);
       }
     }
 
@@ -107,7 +103,7 @@ export class LanguageStrategy {
         // fail to npm install @microsoft/teamsfx on azure web app, so pack it locally.
         await utils.execute("npm install @microsoft/teamsfx", packDir);
       } catch (e) {
-        throw new CommandExecutionError(`${Commands.NPM_INSTALL}`, e.message, e);
+        throw new CommandExecutionError(`${Commands.NPM_INSTALL}`, e);
       }
     }
   }

diff --git a/packages/fx-core/src/plugins/resource/bot/plugin.ts b/packages/fx-core/src/plugins/resource/bot/plugin.ts
@@ -398,9 +398,16 @@ export class TeamsBotImpl {
         // Remind end developers to update message endpoint manually.
         await DialogUtils.showAndHelp(
           context,
-          `Before running this bot, please manually update bot's message endpoint(${this.config.provision.siteEndpoint}${CommonStrings.MESSAGE_ENDPOINT_SUFFIX}). Click 'Get Help' button for more details.`,
+          Messages.RemindUsersToUpdateMessageEndpoint(
+            `${this.config.provision.siteEndpoint}${CommonStrings.MESSAGE_ENDPOINT_SUFFIX}`
+          ),
           Links.UPDATE_MESSAGE_ENDPOINT
         );
+        Logger.info(
+          Messages.RemindUsersToUpdateMessageEndpoint(
+            `${this.config.provision.siteEndpoint}${CommonStrings.MESSAGE_ENDPOINT_SUFFIX}`
+          )
+        );
         break;
       }
     }

diff --git a/packages/fx-core/src/plugins/resource/bot/resources/messages.ts b/packages/fx-core/src/plugins/resource/bot/resources/messages.ts
@@ -3,7 +3,7 @@
 
 export class Messages {
   public static readonly SomethingIsInvalidWithValue = (something: string, value: string): string =>
-    `'${something}' is invalid with '${value}'.`;
+    `'${something}' is invalid with value '${value}'.`;
   public static readonly InputValidValueForSomething = (something: string): string =>
     `Please select valid values for '${something}'.`;
   public static readonly SomethingIsMissing = (something: string): string =>
@@ -32,12 +32,12 @@ export class Messages {
     "Please click 'Get Help' button for more details.";
   public static readonly ClickIssueButtonToReportIssue =
     "Please click 'Report Issue' button to report the issue.";
-  public static readonly CommandFailWithMessage = (command: string, message: string): string =>
-    `Run '${command}' failed with message: ${message}`;
+  public static readonly CommandExecutionFailed = (command: string): string =>
+    `Failed to run '${command}'.`;
   public static readonly DoSthBeforeSth = (sth: string, beforeSth: string): string =>
     `Perform command '${sth}' before '${beforeSth}'.`;
-  public static readonly FailToCallAppStudio = (apiName: string): string =>
-    `Failed to execute '${apiName}'.`;
+  public static readonly FailToCallAppStudioForCheckingAADApp =
+    "Failed to call App Studio's api to check AAD application's existence.";
   public static readonly SuccessfullyRetrievedTemplateZip = (zipUrl: string): string =>
     `Successfully retrieved zip package from ${zipUrl}.`;
   public static readonly FallingBackToUseLocalTemplateZip =
@@ -96,6 +96,8 @@ export class Messages {
     "The subscription didn't register to use namespace 'Microsoft.BotService'.";
   public static readonly MaxFreeAppServicePlanIsTen =
     "The maximum number of Free App Service Plan allowed in a Subscription is 10.";
+  public static readonly RemindUsersToUpdateMessageEndpoint = (messageEndpoint: string): string =>
+    `Before running this bot, please manually update bot's message endpoint(${messageEndpoint}). Click 'Get Help' button for more details.`;
 
   // Suggestions
   public static readonly RetryTheCurrentStep = "Please retry the current step.";
@@ -107,4 +109,10 @@ export class Messages {
   public static readonly RecreateTheProject = "Please recreate the project.";
   public static readonly CheckCommandOutputAndTryToFixIt =
     "Please check the command output and try to fix it.";
+  public static readonly DeleteExistingBotChannelRegistration =
+    "Please delete existing azure bot channel registrations.";
+  public static readonly DeleteBotAfterAzureAccountSwitching =
+    "If azure account is switched, don't forget to delete azure bot channel registration under the previous account.";
+  public static readonly CheckOutputLogAndTryToFix =
+    "Please check log in output channel and try to fix this issue.";
 }
diff --git a/packages/fx-core/tests/plugins/resource/bot/unit/errors.test.ts b/packages/fx-core/tests/plugins/resource/bot/unit/errors.test.ts
@@ -7,7 +7,7 @@ import {
   PluginError,
   ErrorType,
   UserInputsError,
-  CallAppStudioError,
+  AADAppCheckingError,
   ConfigUpdatingError,
   ConfigValidationError,
   PackDirExistenceError,
@@ -46,13 +46,11 @@ describe("Test Errors", () => {
     });
   });
 
-  describe("CallAppStudioError", () => {
+  describe("AADAppCheckingError", () => {
     it("Happy Path", () => {
       // Arrange
-      const apiName = "genPassword";
-
       // Act
-      const myError = new CallAppStudioError(apiName);
+      const myError = new AADAppCheckingError();
 
       // Assert
       chai.assert.isTrue(myError instanceof PluginError);
@@ -70,7 +68,7 @@ describe("Test Errors", () => {
 
       // Assert
       chai.assert.isTrue(myError instanceof PluginError);
-      chai.assert.isTrue(myError.errorType === ErrorType.System);
+      chai.assert.isTrue(myError.errorType === ErrorType.User);
     });
   });