Better handling throttling error in bulk delete (#1954)

* Implement retry on throttling for nosql

* Clean up code

* Produce specific error for throttling error in mongoProxy bulk delete. Clean up code.

* Fix throttling doc url

* Fix mongo error wording

* Fix unit test

* Unit test cleanup

* Fix format

* Fix unit tests

* Fix format

* Fix unit test

* Fix format

* Improve comments

* Improve error message wording. Fix URL and add specific URL for Mongo and NoSql.

* Fix error messages. Add console errors.

* Clean up selection of various delete fct

* Fix error display

src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.test.tsx

@@ -1,7 +1,10 @@
 import { FeedResponse, ItemDefinition, Resource } from "@azure/cosmos";
+import { waitFor } from "@testing-library/react";
 import { deleteDocuments } from "Common/dataAccess/deleteDocument";
 import { Platform, updateConfigContext } from "ConfigContext";
+import { useDialog } from "Explorer/Controls/Dialog";
 import { EditorReactProps } from "Explorer/Controls/Editor/EditorReact";
+import { ProgressModalDialog } from "Explorer/Controls/ProgressModalDialog";
 import { useCommandBar } from "Explorer/Menus/CommandBar/CommandBarComponentAdapter";
 import {
   ButtonsDependencies,
@@ -65,12 +68,14 @@ jest.mock("Explorer/Controls/Editor/EditorReact", () => ({
   EditorReact: (props: EditorReactProps) => <>{props.content}</>,
 }));
 
+const mockDialogState = {
+  showOkCancelModalDialog: jest.fn((title: string, subText: string, okLabel: string, onOk: () => void) => onOk()),
+  showOkModalDialog: () => {},
+};
+
 jest.mock("Explorer/Controls/Dialog", () => ({
   useDialog: {
-    getState: jest.fn(() => ({
-      showOkCancelModalDialog: (title: string, subText: string, okLabel: string, onOk: () => void) => onOk(),
-      showOkModalDialog: () => {},
-    })),
+    getState: jest.fn(() => mockDialogState),
   },
 }));
 
@@ -80,6 +85,10 @@ jest.mock("Common/dataAccess/deleteDocument", () => ({
   ),
 }));
 
+jest.mock("Explorer/Controls/ProgressModalDialog", () => ({
+  ProgressModalDialog: jest.fn(() => <></>),
+}));
+
 async function waitForComponentToPaint<P = unknown>(wrapper: ReactWrapper<P> | ShallowWrapper<P>, amount = 0) {
   let newWrapper;
   await act(async () => {
@@ -469,7 +478,29 @@ describe("Documents tab (noSql API)", () => {
       expect(useCommandBar.getState().contextButtons.find((button) => button.id === DISCARD_BUTTON_ID)).toBeDefined();
     });
 
-    it("clicking Delete Document asks for confirmation", () => {
+    it("clicking Delete Document asks for confirmation", async () => {
+      act(async () => {
+        await useCommandBar
+          .getState()
+          .contextButtons.find((button) => button.id === DELETE_BUTTON_ID)
+          .onCommandClick(undefined);
+      });
+
+      expect(useDialog.getState().showOkCancelModalDialog).toHaveBeenCalled();
+    });
+
+    it("clicking Delete Document for NoSql shows progress dialog", () => {
+      act(() => {
+        useCommandBar
+          .getState()
+          .contextButtons.find((button) => button.id === DELETE_BUTTON_ID)
+          .onCommandClick(undefined);
+      });
+
+      expect(ProgressModalDialog).toHaveBeenCalled();
+    });
+
+    it("clicking Delete Document eventually calls delete client api", () => {
       const mockDeleteDocuments = deleteDocuments as jest.Mock;
       mockDeleteDocuments.mockClear();
 
@@ -480,7 +511,8 @@ describe("Documents tab (noSql API)", () => {
           .onCommandClick(undefined);
       });
 
-      expect(mockDeleteDocuments).toHaveBeenCalled();
+      // The implementation uses setTimeout, so wait for it to finish
+      waitFor(() => expect(mockDeleteDocuments).toHaveBeenCalled());
     });
   });
 });

src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2.tsx

@@ -1,5 +1,15 @@
 import { Item, ItemDefinition, PartitionKey, PartitionKeyDefinition, QueryIterator, Resource } from "@azure/cosmos";
-import { Button, Input, TableRowId, makeStyles, shorthands } from "@fluentui/react-components";
+import {
+  Button,
+  Input,
+  Link,
+  MessageBar,
+  MessageBarBody,
+  MessageBarTitle,
+  TableRowId,
+  makeStyles,
+  shorthands,
+} from "@fluentui/react-components";
 import { Dismiss16Filled } from "@fluentui/react-icons";
 import { KeyCodes, QueryCopilotSampleContainerId, QueryCopilotSampleDatabaseId } from "Common/Constants";
 import { getErrorMessage, getErrorStack } from "Common/ErrorHandlingUtils";
@@ -16,6 +26,7 @@ import { Platform, configContext } from "ConfigContext";
 import { CommandButtonComponentProps } from "Explorer/Controls/CommandButton/CommandButtonComponent";
 import { useDialog } from "Explorer/Controls/Dialog";
 import { EditorReact } from "Explorer/Controls/Editor/EditorReact";
+import { ProgressModalDialog } from "Explorer/Controls/ProgressModalDialog";
 import Explorer from "Explorer/Explorer";
 import { useCommandBar } from "Explorer/Menus/CommandBar/CommandBarComponentAdapter";
 import { querySampleDocuments, readSampleDocument } from "Explorer/QueryCopilot/QueryCopilotUtilities";
@@ -35,7 +46,7 @@ import { QueryConstants } from "Shared/Constants";
 import { LocalStorageUtility, StorageKey } from "Shared/StorageUtility";
 import { Action } from "Shared/Telemetry/TelemetryConstants";
 import { userContext } from "UserContext";
-import { logConsoleError } from "Utils/NotificationConsoleUtils";
+import { logConsoleError, logConsoleInfo } from "Utils/NotificationConsoleUtils";
 import { Allotment } from "allotment";
 import React, { KeyboardEventHandler, useCallback, useEffect, useMemo, useRef, useState } from "react";
 import { format } from "react-string-format";
@@ -60,6 +71,9 @@ import TabsBase from "../TabsBase";
 import { ColumnDefinition, DocumentsTableComponent, DocumentsTableComponentItem } from "./DocumentsTableComponent";
 
 const MAX_FILTER_HISTORY_COUNT = 100; // Datalist will become scrollable, so we can afford to keep more items than fit on the screen
+const NO_SQL_THROTTLING_DOC_URL =
+  "https://learn.microsoft.com/azure/cosmos-db/nosql/troubleshoot-request-rate-too-large";
+const MONGO_THROTTLING_DOC_URL = "https://learn.microsoft.com/azure/cosmos-db/mongodb/prevent-rate-limiting-errors";
 
 const loadMoreHeight = LayoutConstants.rowHeight;
 export const useDocumentsTabStyles = makeStyles({
@@ -110,6 +124,20 @@ export const useDocumentsTabStyles = makeStyles({
       ...shorthands.outline("1px", "dotted"),
     },
   },
+  floatingControlsContainer: {
+    position: "relative",
+  },
+  floatingControls: {
+    position: "absolute",
+    top: "6px",
+    right: 0,
+    float: "right",
+    backgroundColor: "white",
+    zIndex: 1,
+  },
+  deleteProgressContent: {
+    paddingTop: tokens.spacingVerticalL,
+  },
 });
 
 export class DocumentsTabV2 extends TabsBase {
@@ -609,6 +637,23 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
     readSubComponentState<FilterHistory>(SubComponentName.FilterHistory, _collection, [] as FilterHistory),
   );
 
+  // For progress bar for bulk delete (noSql)
+  const [isBulkDeleteDialogOpen, setIsBulkDeleteDialogOpen] = React.useState(false);
+  const [bulkDeleteProcess, setBulkDeleteProcess] = useState<{
+    pendingIds: DocumentId[];
+    successfulIds: DocumentId[];
+    throttledIds: DocumentId[];
+    failedIds: DocumentId[];
+    beforeExecuteMs: number; // Delay before executing delete. Used for retrying throttling after a specified delay
+  }>(undefined);
+  const [bulkDeleteOperation, setBulkDeleteOperation] = useState<{
+    onCompleted: (documentIds: DocumentId[]) => void;
+    onFailed: (reason?: unknown) => void;
+    count: number;
+    collection: CollectionBase;
+  }>(undefined);
+  const [bulkDeleteMode, setBulkDeleteMode] = useState<"inProgress" | "completed" | "aborting" | "aborted">(undefined);
+
   const setKeyboardActions = useKeyboardActionGroup(KeyboardActionGroup.ACTIVE_TAB);
 
   useEffect(() => {
@@ -634,6 +679,97 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
     }
   }, [documentIds, clickedRowIndex, editorState]);
 
+  /**
+   * Recursively delete all documents by retrying throttled requests (429).
+   * This only works for NoSQL, because the bulk response includes status for each delete document request.
+   * Recursion is implemented using React useEffect (as opposed to recursively calling setTimeout), because it
+   * has to update the <ProgressModalDialog> or check if the user is aborting the operation via state React
+   * variables.
+   *
+   * Inputs are the bulkDeleteOperation, bulkDeleteProcess and bulkDeleteMode state variables.
+   * When the bulkDeleteProcess changes, the function in the useEffect is triggered and checks if the process
+   * was aborted or completed, which will resolve the promise.
+   * Otherwise, it will attempt to delete documents of the pending and throttled ids arrays.
+   * Once deletion is completed, the function updates bulkDeleteProcess with the results, which will trigger
+   * the function to be called again.
+   */
+  useEffect(() => {
+    if (!bulkDeleteOperation || !bulkDeleteProcess || !bulkDeleteMode) {
+      return;
+    }
+
+    if (bulkDeleteMode === "completed" || bulkDeleteMode === "aborted") {
+      // no op in the case function is called again
+      return;
+    }
+
+    if (
+      (bulkDeleteProcess.pendingIds.length === 0 && bulkDeleteProcess.throttledIds.length === 0) ||
+      bulkDeleteMode === "aborting"
+    ) {
+      // Successfully deleted all documents or operation was aborted
+      bulkDeleteOperation.onCompleted(bulkDeleteProcess.successfulIds);
+      setBulkDeleteMode(bulkDeleteMode === "aborting" ? "aborted" : "completed");
+      return;
+    }
+
+    // Start deleting documents or retry throttled requests
+    const newPendingIds = bulkDeleteProcess.pendingIds.concat(bulkDeleteProcess.throttledIds);
+    const timeout = bulkDeleteProcess.beforeExecuteMs || 0;
+
+    setTimeout(() => {
+      deleteNoSqlDocuments(bulkDeleteOperation.collection, [...newPendingIds])
+        .then((deleteResult) => {
+          let retryAfterMilliseconds = 0;
+          const newSuccessful: DocumentId[] = [];
+          const newThrottled: DocumentId[] = [];
+          const newFailed: DocumentId[] = [];
+          deleteResult.forEach((result) => {
+            if (result.statusCode === Constants.HttpStatusCodes.NoContent) {
+              newSuccessful.push(result.documentId);
+            } else if (result.statusCode === Constants.HttpStatusCodes.TooManyRequests) {
+              newThrottled.push(result.documentId);
+              retryAfterMilliseconds = Math.max(result.retryAfterMilliseconds, retryAfterMilliseconds);
+            } else if (result.statusCode >= 400) {
+              newFailed.push(result.documentId);
+              logConsoleError(
+                `Failed to delete document ${result.documentId.id} with status code ${result.statusCode}`,
+              );
+            }
+          });
+
+          logConsoleInfo(`Successfully deleted ${newSuccessful.length} document(s)`);
+
+          if (newThrottled.length > 0) {
+            logConsoleError(
+              `Failed to delete ${newThrottled.length} document(s) due to "Request too large" (429) error. Retrying...`,
+            );
+          }
+
+          // Update result of the bulk delete: method is called again, because the state variables changed
+          // it will decide at the next call what to do
+          setBulkDeleteProcess((prev) => ({
+            pendingIds: [],
+            successfulIds: prev.successfulIds.concat(newSuccessful),
+            throttledIds: newThrottled,
+            failedIds: prev.failedIds.concat(newFailed),
+            beforeExecuteMs: retryAfterMilliseconds,
+          }));
+        })
+        .catch((error) => {
+          console.error("Error deleting documents", error);
+          setBulkDeleteProcess((prev) => ({
+            pendingIds: [],
+            throttledIds: [],
+            successfulIds: prev.successfulIds,
+            failedIds: prev.failedIds.concat(prev.pendingIds),
+            beforeExecuteMs: undefined,
+          }));
+          bulkDeleteOperation.onFailed(error);
+        });
+    }, timeout);
+  }, [bulkDeleteOperation, bulkDeleteProcess, bulkDeleteMode]);
+
   const applyFilterButton = {
     enabled: true,
     visible: true,
@@ -983,8 +1119,35 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
     setSelectedDocumentContent(selectedDocumentContentBaseline);
   }, [initialDocumentContent, selectedDocumentContentBaseline, setSelectedDocumentContent]);
 
+  /**
+   * Trigger a useEffect() to bulk delete noSql documents
+   * @param collection
+   * @param documentIds
+   * @returns
+   */
+  const _bulkDeleteNoSqlDocuments = (collection: CollectionBase, documentIds: DocumentId[]): Promise<DocumentId[]> =>
+    new Promise<DocumentId[]>((resolve, reject) => {
+      setBulkDeleteOperation({
+        onCompleted: resolve,
+        onFailed: reject,
+        count: documentIds.length,
+        collection,
+      });
+      setBulkDeleteProcess({
+        pendingIds: [...documentIds],
+        throttledIds: [],
+        successfulIds: [],
+        failedIds: [],
+        beforeExecuteMs: 0,
+      });
+      setIsBulkDeleteDialogOpen(true);
+      setBulkDeleteMode("inProgress");
+    });
+
   /**
    * Implementation using bulk delete NoSQL API
+   * @param list of document ids to delete
+   * @returns Promise of list of deleted document ids
    */
   const _deleteDocuments = useCallback(
     async (toDeleteDocumentIds: DocumentId[]): Promise<DocumentId[]> => {
@@ -995,39 +1158,44 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
       });
       setIsExecuting(true);
 
-      // TODO: Once JS SDK Bug fix for bulk deleting legacy containers (whose systemKey==1) is released:
-      // Remove the check for systemKey, remove call to deleteNoSqlDocument(). deleteNoSqlDocuments() should always be called.
-      const _deleteNoSqlDocuments = async (
-        collection: CollectionBase,
-        toDeleteDocumentIds: DocumentId[],
-      ): Promise<DocumentId[]> =>
-        partitionKey.systemKey
-          ? deleteNoSqlDocument(collection, toDeleteDocumentIds[0]).then(() => [toDeleteDocumentIds[0]])
-          : deleteNoSqlDocuments(collection, toDeleteDocumentIds);
-
-      // TODO: Once new mongo proxy is available for all users, remove the call for MongoProxyClient.deleteDocument().
-      // MongoProxyClient.deleteDocuments() should be called for all users.
-      const _deleteMongoDocuments = async (
-        databaseId: string,
-        collection: ViewModels.Collection,
-        documentIds: DocumentId[],
-      ) =>
-        isMongoBulkDeleteDisabled
-          ? MongoProxyClient.deleteDocument(databaseId, collection, documentIds[0]).then(() => [toDeleteDocumentIds[0]])
-          : MongoProxyClient.deleteDocuments(databaseId, collection, documentIds).then(
-              ({ deletedCount, isAcknowledged }) => {
-                if (deletedCount === toDeleteDocumentIds.length && isAcknowledged) {
-                  return toDeleteDocumentIds;
-                }
-                throw new Error(
-                  `Delete failed with deletedCount: ${deletedCount} and isAcknowledged: ${isAcknowledged}`,
-                );
-              },
-            );
-
-      const deletePromise = !isPreferredApiMongoDB
-        ? _deleteNoSqlDocuments(_collection, toDeleteDocumentIds)
-        : _deleteMongoDocuments(_collection.databaseId, _collection as ViewModels.Collection, toDeleteDocumentIds);
+      let deletePromise;
+      if (!isPreferredApiMongoDB) {
+        if (partitionKey.systemKey) {
+          // ----------------------------------------------------------------------------------------------------
+          // TODO: Once JS SDK Bug fix for bulk deleting legacy containers (whose systemKey==1) is released:
+          // Remove the check for systemKey, remove call to deleteNoSqlDocument(). deleteNoSqlDocuments() should
+          // always be called for NoSQL.
+          deletePromise = deleteNoSqlDocument(_collection, toDeleteDocumentIds[0]).then(() => {
+            useDialog.getState().showOkModalDialog("Delete document", "Document successfully deleted.");
+            return [toDeleteDocumentIds[0]];
+          });
+          // ----------------------------------------------------------------------------------------------------
+        } else {
+          deletePromise = _bulkDeleteNoSqlDocuments(_collection, toDeleteDocumentIds);
+        }
+      } else {
+        if (isMongoBulkDeleteDisabled) {
+          // TODO: Once new mongo proxy is available for all users, remove the call for MongoProxyClient.deleteDocument().
+          // MongoProxyClient.deleteDocuments() should be called for all users.
+          deletePromise = MongoProxyClient.deleteDocument(
+            _collection.databaseId,
+            _collection as ViewModels.Collection,
+            toDeleteDocumentIds[0],
+          ).then(() => [toDeleteDocumentIds[0]]);
+          // ----------------------------------------------------------------------------------------------------
+        } else {
+          deletePromise = MongoProxyClient.deleteDocuments(
+            _collection.databaseId,
+            _collection as ViewModels.Collection,
+            toDeleteDocumentIds,
+          ).then(({ deletedCount, isAcknowledged }) => {
+            if (deletedCount === toDeleteDocumentIds.length && isAcknowledged) {
+              return toDeleteDocumentIds;
+            }
+            throw new Error(`Delete failed with deletedCount: ${deletedCount} and isAcknowledged: ${isAcknowledged}`);
+          });
+        }
+      }
 
       return deletePromise
         .then(
@@ -1058,9 +1226,11 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
             throw error;
           },
         )
-        .finally(() => setIsExecuting(false));
+        .finally(() => {
+          setIsExecuting(false);
+        });
     },
-    [_collection, isPreferredApiMongoDB, onExecutionErrorChange, tabTitle],
+    [_collection, isPreferredApiMongoDB, onExecutionErrorChange, tabTitle, partitionKey.systemKey],
   );
 
   const deleteDocuments = useCallback(
@@ -1078,14 +1248,25 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
             setClickedRowIndex(undefined);
             setSelectedRows(new Set());
             setEditorState(ViewModels.DocumentExplorerState.noDocumentSelected);
-            useDialog
-              .getState()
-              .showOkModalDialog("Delete documents", `${deletedIds.length} document(s) successfully deleted.`);
           },
-          (error: Error) =>
-            useDialog
-              .getState()
-              .showOkModalDialog("Delete documents", `Deleting document(s) failed (${error.message})`),
+          (error: Error) => {
+            if (error instanceof MongoProxyClient.ThrottlingError) {
+              useDialog
+                .getState()
+                .showOkModalDialog(
+                  "Delete documents",
+                  `Some documents failed to delete due to a rate limiting error. Please try again later. To prevent this in the future, consider increasing the throughput on your container or database.`,
+                  {
+                    linkText: "Learn More",
+                    linkUrl: MONGO_THROTTLING_DOC_URL,
+                  },
+                );
+            } else {
+              useDialog
+                .getState()
+                .showOkModalDialog("Delete documents", `Deleting document(s) failed (${error.message})`);
+            }
+          },
         )
         .finally(() => setIsExecuting(false));
     },
@@ -1870,6 +2051,26 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
     [createIterator, filterContent],
   );
 
+  /**
+   * While retrying, display: retrying now.
+   * If completed and all documents were deleted, display: all documents deleted.
+   * @returns 429 warning message
+   */
+  const get429WarningMessageNoSql = (): string => {
+    let message = 'Some delete requests failed due to a "Request too large" exception (429)';
+
+    if (bulkDeleteOperation.count === bulkDeleteProcess.successfulIds.length) {
+      message += ", but were successfully retried.";
+    } else if (bulkDeleteMode === "inProgress" || bulkDeleteMode === "aborting") {
+      message += ". Retrying now.";
+    } else {
+      message += ".";
+    }
+
+    return (message +=
+      " To prevent this in the future, consider increasing the throughput on your container or database.");
+  };
+
   const onColumnSelectionChange = (newSelectedColumnIds: string[]): void => {
     // Do not allow to unselecting all columns
     if (newSelectedColumnIds.length === 0) {
@@ -2076,6 +2277,50 @@ export const DocumentsTabComponent: React.FunctionComponent<IDocumentsTabCompone
           </Allotment>
         </div>
       </div>
+      {bulkDeleteOperation && (
+        <ProgressModalDialog
+          isOpen={isBulkDeleteDialogOpen}
+          dismissText="Abort"
+          onDismiss={() => {
+            setIsBulkDeleteDialogOpen(false);
+            setBulkDeleteOperation(undefined);
+          }}
+          onCancel={() => setBulkDeleteMode("aborting")}
+          title={`Deleting ${bulkDeleteOperation.count} document(s)`}
+          message={`Successfully deleted ${bulkDeleteProcess.successfulIds.length} document(s).`}
+          maxValue={bulkDeleteOperation.count}
+          value={bulkDeleteProcess.successfulIds.length}
+          mode={bulkDeleteMode}
+        >
+          <div className={styles.deleteProgressContent}>
+            {(bulkDeleteMode === "aborting" || bulkDeleteMode === "aborted") && (
+              <div style={{ paddingBottom: tokens.spacingVerticalL }}>Deleting document(s) was aborted.</div>
+            )}
+            {(bulkDeleteProcess.failedIds.length > 0 ||
+              (bulkDeleteProcess.throttledIds.length > 0 && bulkDeleteMode !== "inProgress")) && (
+              <MessageBar intent="error" style={{ marginBottom: tokens.spacingVerticalL }}>
+                <MessageBarBody>
+                  <MessageBarTitle>Error</MessageBarTitle>
+                  Failed to delete{" "}
+                  {bulkDeleteMode === "inProgress"
+                    ? bulkDeleteProcess.failedIds.length
+                    : bulkDeleteProcess.failedIds.length + bulkDeleteProcess.throttledIds.length}{" "}
+                  document(s).
+                </MessageBarBody>
+              </MessageBar>
+            )}
+            <MessageBar intent="warning">
+              <MessageBarBody>
+                <MessageBarTitle>Warning</MessageBarTitle>
+                {get429WarningMessageNoSql()}{" "}
+                <Link href={NO_SQL_THROTTLING_DOC_URL} target="_blank">
+                  Learn More
+                </Link>
+              </MessageBarBody>
+            </MessageBar>
+          </div>
+        </ProgressModalDialog>
+      )}
     </CosmosFluentProvider>
   );
 };

src/Explorer/Tabs/DocumentsTabV2/DocumentsTabV2Mongo.test.tsx

@@ -49,7 +49,8 @@ jest.mock("Common/MongoProxyClient", () => ({
       id: "id1",
     }),
   ),
-  deleteDocuments: jest.fn(() => Promise.resolve()),
+  deleteDocuments: jest.fn(() => Promise.resolve({ deleteCount: 0, isAcknowledged: true })),
+  ThrottlingError: Error,
   useMongoProxyEndpoint: jest.fn(() => true),
 }));
 
@@ -179,7 +180,7 @@ describe("Documents tab (Mongo API)", () => {
       expect(useCommandBar.getState().contextButtons.find((button) => button.id === DISCARD_BUTTON_ID)).toBeDefined();
     });
 
-    it("clicking Delete Document asks for confirmation", () => {
+    it("clicking Delete Document eventually calls delete client api", () => {
       const mockDeleteDocuments = deleteDocuments as jest.Mock;
       mockDeleteDocuments.mockClear();