refactor error handling part 1 (#307)

- created `getErrorMessage` function which takes in an error string or any type of error object and returns the correct error message
- replaced `error.message` with `getErrorMessage` since `error` could be a string in some cases
- merged sendNotificationForError.ts with ErrorHandlingUtils.ts
- some minor refactoring

In part 2, I will make the following changes:
 - Make `Logger.logError` function take an error message string instead of an error object. This will reduce some redundancy where the `getErrorMessage` function is being called twice (the error object passed by the caller is already an error message).
 - Update every `TelemetryProcessor.traceFailure` call to make sure we pass in an error message instead of an error object since we stringify the data we send.

src/Common/CosmosClient.ts

@@ -1,6 +1,7 @@
 import * as Cosmos from "@azure/cosmos";
 import { RequestInfo, setAuthorizationTokenHeaderUsingMasterKey } from "@azure/cosmos";
 import { configContext, Platform } from "../ConfigContext";
+import { getErrorMessage } from "./ErrorHandlingUtils";
 import { logConsoleError } from "../Utils/NotificationConsoleUtils";
 import { EmulatorMasterKey, HttpHeaders } from "./Constants";
 import { userContext } from "../UserContext";
@@ -69,7 +70,7 @@ export async function getTokenFromAuthService(verb: string, resourceType: string
     const result = JSON.parse(await response.json());
     return result;
   } catch (error) {
-    logConsoleError(`Failed to get authorization headers for ${resourceType}: ${error.message}`);
+    logConsoleError(`Failed to get authorization headers for ${resourceType}: ${getErrorMessage(error)}`);
     return Promise.reject(error);
   }
 }

src/Common/ErrorHandlingUtils.ts

@@ -1,11 +1,52 @@
-import { CosmosError, sendNotificationForError } from "./dataAccess/sendNotificationForError";
+import { HttpStatusCodes } from "./Constants";
+import { MessageTypes } from "../Contracts/ExplorerContracts";
+import { SubscriptionType } from "../Contracts/ViewModels";
 import { logConsoleError } from "../Utils/NotificationConsoleUtils";
 import { logError } from "./Logger";
-import { replaceKnownError } from "./ErrorParserUtility";
+import { sendMessage } from "./MessageHandler";
 
-export const handleError = (error: CosmosError, consoleErrorPrefix: string, area: string): void => {
-  const sanitizedErrorMsg = replaceKnownError(error.message);
-  logConsoleError(`${consoleErrorPrefix}:\n ${sanitizedErrorMsg}`);
-  logError(sanitizedErrorMsg, area, error.code);
-  sendNotificationForError(error);
+export interface CosmosError {
+  code: number;
+  message?: string;
+}
+
+export const handleError = (error: string | CosmosError, consoleErrorPrefix: string, area: string): void => {
+  const errorMessage = getErrorMessage(error);
+  const errorCode = typeof error === "string" ? undefined : error.code;
+  // logs error to data explorer console
+  logConsoleError(`${consoleErrorPrefix}:\n ${errorMessage}`);
+  // logs error to both app insight and kusto
+  logError(errorMessage, area, errorCode);
+  // checks for errors caused by firewall and sends them to portal to handle
+  sendNotificationForError(errorMessage, errorCode);
+};
+
+export const getErrorMessage = (error: string | CosmosError | Error): string => {
+  const errorMessage = typeof error === "string" ? error : error.message;
+  return replaceKnownError(errorMessage);
+};
+
+const sendNotificationForError = (errorMessage: string, errorCode: number): void => {
+  if (errorCode === HttpStatusCodes.Forbidden) {
+    if (errorMessage?.toLowerCase().indexOf("sharedoffer is disabled for your account") > 0) {
+      return;
+    }
+    sendMessage({
+      type: MessageTypes.ForbiddenError,
+      reason: errorMessage
+    });
+  }
+};
+
+const replaceKnownError = (errorMessage: string): string => {
+  if (
+    window.dataExplorer?.subscriptionType() === SubscriptionType.Internal &&
+    errorMessage.indexOf("SharedOffer is Disabled for your account") >= 0
+  ) {
+    return "Database throughput is not supported for internal subscriptions.";
+  } else if (errorMessage.indexOf("Partition key paths must contain only valid") >= 0) {
+    return "Partition key paths must contain only valid characters and not contain a trailing slash or wildcard character.";
+  }
+
+  return errorMessage;
 };

src/Common/ErrorParserUtility.ts

@@ -1,18 +1,4 @@
 import * as DataModels from "../Contracts/DataModels";
-import * as ViewModels from "../Contracts/ViewModels";
-
-export function replaceKnownError(err: string): string {
-  if (
-    window.dataExplorer.subscriptionType() === ViewModels.SubscriptionType.Internal &&
-    err.indexOf("SharedOffer is Disabled for your account") >= 0
-  ) {
-    return "Database throughput is not supported for internal subscriptions.";
-  } else if (err.indexOf("Partition key paths must contain only valid") >= 0) {
-    return "Partition key paths must contain only valid characters and not contain a trailing slash or wildcard character.";
-  }
-
-  return err;
-}
 
 export function parse(err: any): DataModels.ErrorDataModel[] {
   try {

src/Common/Logger.ts

@@ -1,3 +1,4 @@
+import { CosmosError, getErrorMessage } from "./ErrorHandlingUtils";
 import { sendMessage } from "./MessageHandler";
 import { Diagnostics, MessageTypes } from "../Contracts/ExplorerContracts";
 import { appInsights } from "../Shared/appInsights";
@@ -21,14 +22,9 @@ export function logWarning(message: string, area: string, code?: number): void {
   return _logEntry(entry);
 }
 
-export function logError(message: string | Error, area: string, code?: number): void {
-  let logMessage: string;
-  if (typeof message === "string") {
-    logMessage = message;
-  } else {
-    logMessage = JSON.stringify(message, Object.getOwnPropertyNames(message));
-  }
-  const entry: Diagnostics.LogEntry = _generateLogEntry(Diagnostics.LogEntryLevel.Error, logMessage, area, code);
+export function logError(error: string | CosmosError | Error, area: string, code?: number): void {
+  const errorMessage: string = getErrorMessage(error);
+  const entry: Diagnostics.LogEntry = _generateLogEntry(Diagnostics.LogEntryLevel.Error, errorMessage, area, code);
   return _logEntry(entry);
 }
 

src/Common/QueriesClient.ts

@@ -12,6 +12,7 @@ import { BackendDefaults, HttpStatusCodes, SavedQueries } from "./Constants";
 import { userContext } from "../UserContext";
 import { createDocument, deleteDocument, queryDocuments, queryDocumentsPage } from "./DocumentClientUtilityBase";
 import { createCollection } from "./dataAccess/createCollection";
+import { handleError } from "./ErrorHandlingUtils";
 import * as ErrorParserUtility from "./ErrorParserUtility";
 import * as Logger from "./Logger";
 
@@ -53,13 +54,8 @@ export class QueriesClient {
           return Promise.resolve(collection);
         },
         (error: any) => {
-          const stringifiedError: string = error.message;
-          NotificationConsoleUtils.logConsoleMessage(
-            ConsoleDataType.Error,
-            `Failed to set up account for saving queries: ${stringifiedError}`
-          );
-          Logger.logError(stringifiedError, "setupQueriesCollection");
-          return Promise.reject(stringifiedError);
+          handleError(error, "Failed to set up account for saving queries", "setupQueriesCollection");
+          return Promise.reject(error);
         }
       )
       .finally(() => NotificationConsoleUtils.clearInProgressMessageWithId(id));
@@ -163,25 +159,15 @@ export class QueriesClient {
               return Promise.resolve(queries);
             },
             (error: any) => {
-              const stringifiedError: string = error.message;
-              NotificationConsoleUtils.logConsoleMessage(
-                ConsoleDataType.Error,
-                `Failed to fetch saved queries: ${stringifiedError}`
-              );
-              Logger.logError(stringifiedError, "getSavedQueries");
-              return Promise.reject(stringifiedError);
+              handleError(error, "Failed to fetch saved queries", "getSavedQueries");
+              return Promise.reject(error);
             }
           );
         },
         (error: any) => {
           // should never get into this state but we handle this regardless
-          const stringifiedError: string = error.message;
-          NotificationConsoleUtils.logConsoleMessage(
-            ConsoleDataType.Error,
-            `Failed to fetch saved queries: ${stringifiedError}`
-          );
-          Logger.logError(stringifiedError, "getSavedQueries");
-          return Promise.reject(stringifiedError);
+          handleError(error, "Failed to fetch saved queries", "getSavedQueries");
+          return Promise.reject(error);
         }
       )
       .finally(() => NotificationConsoleUtils.clearInProgressMessageWithId(id));
@@ -232,13 +218,8 @@ export class QueriesClient {
           return Promise.resolve();
         },
         (error: any) => {
-          const stringifiedError: string = error.message;
-          NotificationConsoleUtils.logConsoleMessage(
-            ConsoleDataType.Error,
-            `Failed to delete query ${query.queryName}: ${stringifiedError}`
-          );
-          Logger.logError(stringifiedError, "deleteQuery");
-          return Promise.reject(stringifiedError);
+          handleError(error, `Failed to delete query ${query.queryName}`, "deleteQuery");
+          return Promise.reject(error);
         }
       )
       .finally(() => NotificationConsoleUtils.clearInProgressMessageWithId(id));

src/Common/dataAccess/createTrigger.ts

@@ -7,9 +7,8 @@ import {
 } from "../../Utils/arm/generatedClients/2020-04-01/types";
 import { client } from "../CosmosClient";
 import { createUpdateSqlTrigger, getSqlTrigger } from "../../Utils/arm/generatedClients/2020-04-01/sqlResources";
-import { logConsoleError, logConsoleProgress } from "../../Utils/NotificationConsoleUtils";
-import { logError } from "../Logger";
-import { sendNotificationForError } from "./sendNotificationForError";
+import { handleError } from "../ErrorHandlingUtils";
+import { logConsoleProgress } from "../../Utils/NotificationConsoleUtils";
 import { userContext } from "../../UserContext";
 
 export async function createTrigger(
@@ -66,9 +65,7 @@ export async function createTrigger(
       .scripts.triggers.create(trigger);
     return response.resource;
   } catch (error) {
-    logConsoleError(`Error while creating trigger ${trigger.id}:\n ${error.message}`);
-    logError(error.message, "CreateTrigger", error.code);
-    sendNotificationForError(error);
+    handleError(error, `Error while creating trigger ${trigger.id}`, "CreateTrigger");
     throw error;
   } finally {
     clearMessage();

src/Common/dataAccess/readOffers.ts

@@ -1,7 +1,7 @@
 import { Offer } from "../../Contracts/DataModels";
 import { logConsoleProgress } from "../../Utils/NotificationConsoleUtils";
 import { client } from "../CosmosClient";
-import { handleError } from "../ErrorHandlingUtils";
+import { handleError, getErrorMessage } from "../ErrorHandlingUtils";
 
 export const readOffers = async (): Promise<Offer[]> => {
   const clearMessage = logConsoleProgress(`Querying offers`);
@@ -13,7 +13,7 @@ export const readOffers = async (): Promise<Offer[]> => {
     return response?.resources;
   } catch (error) {
     // This should be removed when we can correctly identify if an account is serverless when connected using connection string too.
-    if (error.message.includes("Reading or replacing offers is not supported for serverless accounts")) {
+    if (getErrorMessage(error)?.includes("Reading or replacing offers is not supported for serverless accounts")) {
       return [];
     }
 

src/Common/dataAccess/sendNotificationForError.ts

@@ -1,20 +0,0 @@
-import * as Constants from "../Constants";
-import { sendMessage } from "../MessageHandler";
-import { MessageTypes } from "../../Contracts/ExplorerContracts";
-
-export interface CosmosError {
-  code: number;
-  message?: string;
-}
-
-export function sendNotificationForError(error: CosmosError): void {
-  if (error && error.code === Constants.HttpStatusCodes.Forbidden) {
-    if (error.message && error.message.toLowerCase().indexOf("sharedoffer is disabled for your account") > 0) {
-      return;
-    }
-    sendMessage({
-      type: MessageTypes.ForbiddenError,
-      reason: error && error.message ? error.message : error
-    });
-  }
-}

src/Common/dataAccess/updateOfferThroughputBeyondLimit.ts

@@ -1,8 +1,9 @@
 import { Platform, configContext } from "../../ConfigContext";
 import { getAuthorizationHeader } from "../../Utils/AuthorizationUtils";
 import { AutoPilotOfferSettings } from "../../Contracts/DataModels";
-import { logConsoleProgress, logConsoleInfo, logConsoleError } from "../../Utils/NotificationConsoleUtils";
+import { logConsoleProgress, logConsoleInfo } from "../../Utils/NotificationConsoleUtils";
 import { HttpHeaders } from "../Constants";
+import { handleError } from "../ErrorHandlingUtils";
 
 interface UpdateOfferThroughputRequest {
   subscriptionId: string;
@@ -44,8 +45,13 @@ export async function updateOfferThroughputBeyondLimit(request: UpdateOfferThrou
     clearMessage();
     return undefined;
   }
+
   const error = await response.json();
-  logConsoleError(`Failed to request an increase in throughput for ${request.throughput}: ${error.message}`);
+  handleError(
+    error,
+    `Failed to request an increase in throughput for ${request.throughput}`,
+    "updateOfferThroughputBeyondLimit"
+  );
   clearMessage();
-  throw new Error(error.message);
+  throw error;
 }