Change how we handle exceptions in driver code: they become non-retry…

…able (#4967)

Closes #4946

All errors that come from storage go through translation where we explicitly assign canRetry property to error object that is thrown.
But if we do not go through that translation (like if code hit exception, like dereferencing undefined), then error object does not have that property and Loader layer will keep retrying.

Switching to fail-fast paradigm here to find these bugs and address them, but also not to retry forever.

packages/loader/container-loader/src/test/retriableDocumentStorageService.spec.ts

@@ -45,6 +45,7 @@ describe("RetriableDocumentStorageService Tests", () => {
                 retryTimes -= 1;
                 const error = new Error("Throw error");
                 (error as any).retryAfterSeconds = 10;
+                (error as any).canRetry = true;
                 throw error;
             }
             return "true";
@@ -84,8 +85,9 @@ describe("RetriableDocumentStorageService Tests", () => {
         internalService.read = async (id: string) => {
             if (retryTimes > 0) {
                 retryTimes -= 1;
-                // eslint-disable-next-line @typescript-eslint/no-throw-literal
-                throw "error";
+                const err = new Error("error");
+                (err as any).canRetry = true;
+                throw err;
             }
             return "true";
         };
@@ -116,6 +118,25 @@ describe("RetriableDocumentStorageService Tests", () => {
         assert.strictEqual(success, "false", "Should not succeed as canRetry was not set");
     });
 
+    it("Should not retry if canRetry is not set", async () => {
+        let retryTimes: number = 1;
+        let success = "false";
+        internalService.read = async (id: string) => {
+            if (retryTimes > 0) {
+                retryTimes -= 1;
+                const error = new Error("error");
+                throw error;
+            }
+            return "true";
+        };
+        try {
+            success = await retriableStorageService.read("");
+            assert.fail("Should not succeed");
+        } catch (error) {}
+        assert.strictEqual(retryTimes, 0, "Should not retry");
+        assert.strictEqual(success, "false", "Should not succeed as canRetry was not set");
+    });
+
     it("Should not retry if it is disabled", async () => {
         let retryTimes: number = 1;
         let success = "false";

packages/loader/driver-utils/src/network.ts

@@ -108,7 +108,8 @@ export function createGenericNetworkError(
 }
 
 /**
- * Check if a connection error can be retried.  Unless explicitly disallowed, retry is allowed.
+ * Check if a connection error can be retried.  Unless explicitly allowed, retry is disallowed.
+ * I.e. asserts or unexpected exceptions in our code result in container failure.
  * @param error - The error to inspect for ability to retry
  */
-export const canRetryOnError = (error: any): boolean => error?.canRetry !== false;
+export const canRetryOnError = (error: any): boolean => error?.canRetry === true;

packages/loader/test-loader-utils/src/mockDeltaStorage.ts

@@ -16,14 +16,17 @@ export class MockDocumentDeltaStorageService implements IDocumentDeltaStorageSer
 
     public async get(from: number, to: number): Promise<IDeltasFetchResult> {
         const messages: ISequencedDocumentMessage[] = [];
-        let index: number = -1;
+        let index: number = 0;
 
         // Find first
-        while (this.messages[++index].sequenceNumber <= from) { }
+        while (index < this.messages.length && this.messages[index].sequenceNumber <= from) {
+            index++;
+        }
 
         // start reading
-        while (++index < this.messages.length && this.messages[++index].sequenceNumber < to) {
+        while (index < this.messages.length && this.messages[index].sequenceNumber < to) {
             messages.push(this.messages[index]);
+            index++;
         }
 
         return { messages, partialResult: false };