fix: improve retry logging (#82)

This PR is a clone of
Unstructured-IO/unstructured-python-client#111

### Summary
- remove false success log in split-page after error hook
- moves success/failure logs to the LoggerHook

Author: christinestraub

src/hooks/custom/LogRetryHook.ts

@@ -0,0 +1,82 @@
+import { AfterErrorContext, AfterErrorHook, AfterSuccessContext, AfterSuccessHook } from "../types";
+
+/**
+ * Represents a hook that logs status and information that the request will be retried
+ * after encountering a 5xx error.
+ */
+export class LoggerHook implements AfterSuccessHook, AfterErrorHook {
+  private retriesCounter: Map<string, number> = new Map();
+
+  /**
+   * Log retries to give users visibility into requests.
+   * @param response - The response object received from the server.
+   * @param error - The error object representing the encountered error.
+   * @param operationID - The unique identifier for the operation being logged.
+   */
+  private logRetries(
+    response: Response | null,
+    error: unknown,
+    operationID: string
+  ): void {
+    if (response && response.status >= 500) {
+      console.warn(
+          "Failed to process a request due to API server error with status code %d. " +
+          "Attempting retry number %d after sleep.",
+          response.status,
+          this.retriesCounter.get(operationID)
+      );
+      if (response.statusText) {
+        console.warn("Server message - %s", response.statusText);
+      }
+    } else if (error) {
+      console.info(
+          `Failed to process a request due to connection error - ${(error as Error).message}. ` +
+          `Attempting retry number ${this.retriesCounter.get(operationID)} after sleep.`
+      );
+    }
+  }
+
+  /**
+   * Handles successful responses, resetting the retry counter for the operation.
+   * Logs a success message indicating that the document was successfully partitioned.
+   * @param hookCtx - The context object containing information about the request.
+   * @param response - The response object received from the server.
+   * @returns The response object.
+   */
+  afterSuccess(hookCtx: AfterSuccessContext, response: Response): Response {
+    this.retriesCounter.delete(hookCtx.operationID);
+    // NOTE: In case of split page partition this means - at least one of the splits was partitioned successfully
+    console.info("Successfully partitioned the document.");
+    return response;
+  }
+
+  /**
+   * Executes after an error occurs during a request.
+   * @param hookCtx - The context object containing information about the request.
+   * @param response - The response object received from the server.
+   * @param error - The error object representing the encountered error.
+   * @returns An object containing the updated response and error.
+   */
+  afterError(
+    hookCtx: AfterErrorContext,
+    response: Response | null,
+    error: unknown
+  ): { response: Response | null; error: unknown } {
+    const currentCount = this.retriesCounter.get(hookCtx.operationID) || 0;
+    this.retriesCounter.set(hookCtx.operationID, currentCount + 1);
+    this.logRetries(response, error, hookCtx.operationID);
+
+    if (response && response.status === 200) {
+      console.info("Successfully partitioned the document.");
+    } else {
+      console.error("Failed to partition the document.");
+      if (response) {
+        console.error(`Server responded with ${response.status} - ${response.statusText}`);
+      }
+      if (error) {
+        console.error(`Following error occurred - ${(error as Error).message}`);
+      }
+    }
+    return { response, error };
+  }
+}

src/hooks/custom/LoggerHook.ts

@@ -213,8 +213,6 @@ export class SplitPdfHook
 
     this.clearOperation(operationID);
 
-    console.info("Successfully processed the request.")
-
     return new Response(body, {
       headers: headers,
       status: response.status,
@@ -241,7 +239,6 @@ export class SplitPdfHook
     const responses = await this.awaitAllRequests(operationID);
 
     if (!responses?.length) {
-      console.error("Failed to process the request.");
       this.clearOperation(operationID);
       return { response, error };
     }
@@ -257,7 +254,6 @@ export class SplitPdfHook
     });
 
     this.clearOperation(operationID);
-    console.info("Successfully processed the request.");
 
     return { response: finalResponse, error: null };
   }

src/hooks/custom/SplitPdfHook.ts

@@ -1,6 +1,6 @@
 import { Hooks } from "./types";
 
-import { LogRetryHook } from "./custom/LogRetryHook";
+import { LoggerHook } from "./custom/LoggerHook";
 import { SplitPdfHook } from "./custom/SplitPdfHook";
 import { HttpsCheckHook } from "./custom/HttpsCheckHook";
 
@@ -16,10 +16,13 @@ export function initHooks(hooks: Hooks) {
   // Hooks are registered per SDK instance, and are valid for the lifetime of the SDK instance
 
   // Initialize hooks
-  const logErrorHook = new LogRetryHook();
+  const loggerHook = new LoggerHook();
   const splitPdfHook = new SplitPdfHook();
   const httpsCheckHook = new HttpsCheckHook();
 
+  // NOTE: logger_hook should stay registered last as logs the status of
+  // request and whether it will be retried which can be changed by e.g. split_pdf_hook
+
   // Register SDK init hooks
   hooks.registerSDKInitHook(httpsCheckHook);
   hooks.registerSDKInitHook(splitPdfHook);
@@ -29,8 +32,9 @@ export function initHooks(hooks: Hooks) {
 
   // Register after success hooks
   hooks.registerAfterSuccessHook(splitPdfHook);
+  hooks.registerAfterSuccessHook(loggerHook)
 
   // Register after error hooks
   hooks.registerAfterErrorHook(splitPdfHook);
-  hooks.registerAfterErrorHook(logErrorHook);
+  hooks.registerAfterErrorHook(loggerHook);
 }

src/hooks/registration.ts

@@ -170,8 +170,9 @@ describe("SplitPdfHook integration tests check splitted file is same as not spli
         });
       } catch (e) {
         if (!expectedOk) {
-          expect(consoleErrorSpy).toHaveBeenCalledTimes(2);
-          expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to process the request.");
+          expect(consoleErrorSpy).toHaveBeenCalledTimes(3);
+          expect(consoleErrorSpy).toHaveBeenCalledWith("Failed to partition the document.");
+          expect(consoleErrorSpy).toHaveBeenCalledWith(expect.stringContaining("Server responded with"));
           expect(consoleWarnSpy).toHaveBeenCalledWith(
             "Attempted to interpret file as pdf, but error arose when splitting by pages. Reverting to non-split pdf handling path."
           );