Introduce normalizeError for getting a valid IFluidErrorBase from an …

…arbitrary caught error (#6764)

This change introduces the `IFluidErrorBase` interface described in #6676, and adds a function `normalizeError`.

`normalizeError` will take any input at all and return a valid `IFluidErrorBase`, whether the original object or a new one if necessary.

It also applies annotations to the resulting error, either telemetry props or an error code to use if there isn't one on the given error.

api-report/container-utils.api.md

@@ -10,6 +10,7 @@ import { IGenericError } from '@fluidframework/container-definitions';
 import { ISequencedDocumentMessage } from '@fluidframework/protocol-definitions';
 import { ITelemetryProperties } from '@fluidframework/common-definitions';
 import { IThrottlingWarning } from '@fluidframework/container-definitions';
+import { IWriteableLoggingError } from '@fluidframework/telemetry-utils';
 import { LoggingError } from '@fluidframework/telemetry-utils';
 
 // @public
@@ -67,7 +68,7 @@ export class ThrottlingWarning extends LoggingError implements IThrottlingWarnin
 }
 
 // @public
-export function wrapError<T>(error: any, newErrorFn: (m: string) => T): T;
+export function wrapError<T extends IWriteableLoggingError>(error: any, newErrorFn: (m: string) => T): T;
 
 
 // (No @packageDocumentation comment for this package)

packages/drivers/odsp-driver/src/epochTracker.ts

@@ -16,7 +16,7 @@ import {
     IPersistedCache,
 } from "@fluidframework/odsp-driver-definitions";
 import { DriverErrorType } from "@fluidframework/driver-definitions";
-import { PerformanceEvent, annotateError } from "@fluidframework/telemetry-utils";
+import { PerformanceEvent, isValidLegacyError } from "@fluidframework/telemetry-utils";
 import { fetchAndParseAsJSONHelper, fetchArray, IOdspResponse } from "./odspUtils";
 import {
     IOdspCache,
@@ -252,14 +252,17 @@ export class EpochTracker implements IPersistedFileCache {
                 // This will only throw if it is an epoch error.
                 this.checkForEpochErrorCore(epochFromResponse, error.errorMessage);
             } catch (epochError) {
-                annotateError(epochError, {
+                assert(isValidLegacyError(epochError),
+                    "epochError expected to be thrown by throwOdspNetworkError and of known type");
+                epochError.addTelemetryProperties({
                     fromCache,
                     clientEpoch: this.fluidEpoch,
                     fetchType,
                 });
                 this.logger.sendErrorEvent({ eventName: "fileOverwrittenInStorage" }, epochError);
                 // If the epoch mismatches, then clear all entries for such file entry from cache.
                 await this.removeEntries();
+                // eslint-disable-next-line @typescript-eslint/no-throw-literal
                 throw epochError;
             }
             // If it was categorized as epoch error but the epoch returned in response matches with the client epoch

packages/loader/container-utils/src/error.ts

@@ -12,18 +12,14 @@ import {
 } from "@fluidframework/container-definitions";
 import {
     isILoggingError,
-    annotateError,
     extractLogSafeErrorProperties,
     LoggingError,
+    IWriteableLoggingError,
+    isValidLegacyError,
 } from "@fluidframework/telemetry-utils";
 import { ITelemetryProperties } from "@fluidframework/common-definitions";
 import { ISequencedDocumentMessage } from "@fluidframework/protocol-definitions";
 
-/** type guard to ensure it has an errorType e.g. via IErrorBase */
-const hasErrorType = (error: any): error is IErrorBase => {
-    return (typeof error?.errorType === "string");
-};
-
 /**
  * Generic wrapper for an unrecognized/uncategorized error object
  */
@@ -102,12 +98,12 @@ export class DataProcessingError extends LoggingError implements IErrorBase {
 
         // Don't coerce if already has an errorType, to distinguish unknown errors from
         // errors that we raised which we already can interpret apart from this classification
-        const error = hasErrorType(originalError)
+        const error = isValidLegacyError(originalError)
             ? originalError
             : wrapError(originalError, newErrorFn);
 
         if (message !== undefined) {
-            annotateError(error, extractSafePropertiesFromMessage(message));
+            error.addTelemetryProperties(extractSafePropertiesFromMessage(message));
         }
         return error;
     }
@@ -133,14 +129,28 @@ export const CreateProcessingError = DataProcessingError.wrapIfUnrecognized;
  * @param props - Properties to include on the error for logging - They will override props on originalError
  */
 export function CreateContainerError(originalError: any, props?: ITelemetryProperties): ICriticalContainerError {
-    const newErrorFn = (errMsg: string) => new GenericError(errMsg, originalError);
-
-    const error = hasErrorType(originalError)
+    const newErrorFn =
+        (errMsg: string) => {
+            // Don't pass in props here, we want to add them last (see below)
+            const newError = new GenericError(errMsg, originalError);
+
+            const { errorType } = extractLogSafeErrorProperties(originalError, false /* sanitizeStack */);
+            if (errorType !== undefined) {
+                // Clobber errorType (which is declared readonly) with the value off the original error
+                Object.assign(newError, { errorType });
+            }
+
+            // By clobbering newError.errorType, we can no longer properly call it a GenericError.
+            // It's still a LoggingError, and does have errorType so it's also IErrorBase
+            return newError as LoggingError & IErrorBase;
+        };
+
+    const error = isValidLegacyError(originalError)
         ? originalError
         : wrapError(originalError, newErrorFn);
 
     if (props !== undefined) {
-        annotateError(error, props);
+        error.addTelemetryProperties(props);
     }
     return error;
 }
@@ -152,18 +162,18 @@ export function CreateContainerError(originalError: any, props?: ITelemetryPrope
  * @param newErrorFn - callback that will create a new error given the original error's message
  * @returns A new error object "wrapping" the given error
  */
-export function wrapError<T>(
+export function wrapError<T extends IWriteableLoggingError>(
     error: any,
     newErrorFn: (m: string) => T,
 ): T {
     const {
         message,
         stack,
-    } = extractLogSafeErrorProperties(error);
+    } = extractLogSafeErrorProperties(error, false /* sanitizeStack */);
     const props = isILoggingError(error) ? error.getTelemetryProperties() : {};
 
     const newError = newErrorFn(message);
-    annotateError(newError, props);
+    newError.addTelemetryProperties(props);
 
     if (stack !== undefined) {
         Object.assign(newError, { stack });

packages/loader/container-utils/src/test/error.spec.ts

@@ -35,12 +35,12 @@ describe("Errors", () => {
             assert(testError.getTelemetryProperties().foo === "bar");
             assert(testError.getTelemetryProperties().message === "womp womp");
         });
-        it("Should preserve existing errorType, and return same object", () => {
-            const originalError = { errorType: "someErrorType" };
+        it("Should preserve existing errorType, but return new object if not a fully valid error", () => {
+            const originalError = { errorType: "someErrorType" }; // missing message and telemetry prop functions
             const testError = CreateContainerError(originalError);
 
             assert(testError.errorType === "someErrorType");
-            assert(testError === originalError);
+            assert(testError !== originalError);
         });
         it("Should ignore non-string errorType", () => {
             const originalError = { errorType: 3 };
@@ -136,13 +136,16 @@ describe("Errors", () => {
 
             assert(coercedError as any === originalError);
         });
-        it("Should skip coercion for object with errorType", () => {
+        it("Should coerce non-LoggingError object with errorType", () => {
             const originalError = {
                 errorType: "Some error type",
             };
             const coercedError = CreateProcessingError(originalError, undefined);
 
-            assert(coercedError as any === originalError);
+            assert(coercedError as any !== originalError);
+            assert(coercedError instanceof DataProcessingError);
+            assert(coercedError.errorType === ContainerErrorType.dataProcessingError);
+            assert(coercedError.message === "[object Object]");
         });
         it("Should coerce LoggingError missing errorType", () => {
             const originalError = new LoggingError(

packages/utils/odsp-doclib-utils/src/odspErrorUtils.ts

@@ -5,7 +5,7 @@
 
 import { ITelemetryProperties } from "@fluidframework/common-definitions";
 import { DriverErrorType } from "@fluidframework/driver-definitions";
-import { annotateError, TelemetryLogger } from "@fluidframework/telemetry-utils";
+import { LoggingError, TelemetryLogger } from "@fluidframework/telemetry-utils";
 import {
     AuthorizationError,
     createGenericNetworkError,
@@ -94,8 +94,8 @@ export function createOdspNetworkError(
     response?: Response,
     responseText?: string,
     props: ITelemetryProperties = {},
-): Error & OdspError & IFacetCodes {
-    let error: Error & OdspError & IFacetCodes;
+): LoggingError & OdspError & IFacetCodes {
+    let error: LoggingError & OdspError & IFacetCodes;
     switch (statusCode) {
         case 400:
             error = new GenericNetworkError(errorMessage, false, { statusCode });
@@ -161,7 +161,7 @@ export function createOdspNetworkError(
 }
 
 export function enrichOdspError(
-    error: OdspError & IFacetCodes,
+    error: LoggingError & OdspError & IFacetCodes,
     response?: Response,
     responseText?: string,
     props: ITelemetryProperties = {},
@@ -183,7 +183,7 @@ export function enrichOdspError(
             props.serverEpoch = response.headers.get("x-fluid-epoch") ?? undefined;
         }
     }
-    annotateError(error, props);
+    error.addTelemetryProperties(props);
     return error;
 }
 

packages/utils/odsp-doclib-utils/src/test/odspErrorUtils.spec.ts

@@ -124,15 +124,15 @@ describe("OdspErrorUtils", () => {
 
     describe("enrichError", () => {
         it("enriched with online flag", () => {
-            const error = new GenericNetworkError("Some message", false) as OdspError & IFacetCodes;
+            const error = new GenericNetworkError("Some message", false) as GenericNetworkError & OdspError;
             enrichOdspError(error);
 
             assert(typeof error.online === "string");
             assert(isILoggingError(error));
             assert(typeof error.getTelemetryProperties().online === "string");
         });
         it("enriched with facetCodes", () => {
-            const error = new GenericNetworkError("Some message", false) as OdspError & IFacetCodes;
+            const error = new GenericNetworkError("Some message", false) as GenericNetworkError & IFacetCodes;
             enrichOdspError(error, undefined /* response */, '{ "error": { "code": "foo" } }' /* responseText */);
 
             assert.deepStrictEqual(error.facetCodes, ["foo"]);
@@ -148,7 +148,7 @@ describe("OdspErrorUtils", () => {
                     return `mock header ${id}`;
                 },
             };
-            const error = new GenericNetworkError("Some message", false) as OdspError & IFacetCodes;
+            const error = new GenericNetworkError("Some message", false);
             enrichOdspError(error, { type: "fooType", headers: mockHeaders } as unknown as Response /* response */, "responseText");
 
             assert.equal((error as any).response, "responseText");

packages/utils/telemetry-utils/package.json

@@ -88,6 +88,7 @@
     "mocha": "^8.4.0",
     "nyc": "^15.0.0",
     "rimraf": "^2.6.2",
+    "sinon": "^7.4.2",
     "typescript": "~4.1.3"
   }
 }