chore: retries improvements (#230)

* chore: retries improvements

* chore: improvements on retries

* chore: improvements on retries

* chore: add unit test

* chore: format

* chore: format

* chore: format

* fix: remove console

.gitignore

@@ -1,5 +1,6 @@
 .vscode
 .env
 .idea
-.lock
+deno.lock
 *.sh
+**/.DS_Store

deps.ts

@@ -6,7 +6,9 @@ import {
 } from "https://deno.land/[email protected]/assert/mod.ts";
 import {
   assertSpyCalls,
+  returnsNext,
   spy,
+  stub,
 } from "https://deno.land/[email protected]/testing/mock.ts";
 
 import { CONSTANTS } from "./src/utils.ts";
@@ -18,4 +20,12 @@ const soxa = new ServiceProvider({
   baseURL,
 });
 
-export { assertEquals, assertRejects, assertSpyCalls, soxa, spy };
+export {
+  assertEquals,
+  assertRejects,
+  assertSpyCalls,
+  returnsNext,
+  soxa,
+  spy,
+  stub,
+};

src/Helpers/Retry.ts

@@ -1,3 +1,4 @@
+import { ServiceError } from "../Types/index.ts";
 import { Logger } from "./Logger.ts";
 
 export type RetryCallbackProps = {
@@ -12,11 +13,17 @@ async function* createAsyncIterable<T>(
   delay: number,
 ) {
   for (let i = 0; i < retries; i++) {
+    const isLastAttempt = i === retries - 1;
     try {
       const data = await callback({ attempt: i });
       yield data;
       return;
     } catch (e) {
+      if (e instanceof ServiceError && isLastAttempt) {
+        yield e;
+        return;
+      }
+
       yield null;
       Logger.error(e);
       await new Promise((resolve) => setTimeout(resolve, delay));
@@ -29,18 +36,27 @@ export class Retry {
   async fetch<T = unknown>(
     callback: callbackType<T>,
   ) {
+    let lastError = null;
     for await (
       const callbackResult of createAsyncIterable<T>(
         callback,
         this.maxRetries,
         this.retryDelay,
       )
     ) {
-      if (callbackResult) {
+      const isError = callbackResult instanceof Error;
+
+      if (callbackResult && !isError) {
         return callbackResult as T;
       }
+
+      if (isError) {
+        lastError = callbackResult;
+      }
     }
 
-    throw new Error(`Max retries (${this.maxRetries}) exceeded.`);
+    throw new Error(`Max retries (${this.maxRetries}) exceeded.`, {
+      cause: lastError,
+    });
   }
 }

src/Services/GithubApiService.ts

@@ -12,13 +12,12 @@ import {
   queryUserPullRequest,
   queryUserRepository,
 } from "../Schemas/index.ts";
-import { soxa } from "../../deps.ts";
 import { Retry } from "../Helpers/Retry.ts";
-import { GithubError, QueryDefaultResponse } from "../Types/index.ts";
 import { CONSTANTS } from "../utils.ts";
 import { EServiceKindError } from "../Types/EServiceKindError.ts";
 import { ServiceError } from "../Types/ServiceError.ts";
 import { Logger } from "../Helpers/Logger.ts";
+import { requestGithubData } from "./request.ts";
 
 // Need to be here - Exporting from another file makes array of null
 export const TOKENS = [
@@ -59,6 +58,7 @@ export class GithubApiService extends GithubRepository {
   async requestUserInfo(username: string): Promise<UserInfo | ServiceError> {
     // Avoid to call others if one of them is null
     const repository = await this.requestUserRepository(username);
+
     if (repository instanceof ServiceError) {
       Logger.error(repository);
       return repository;
@@ -78,7 +78,7 @@ export class GithubApiService extends GithubRepository {
 
     if (status.includes("rejected")) {
       Logger.error(`Can not find a user with username:' ${username}'`);
-      return new ServiceError("not found", EServiceKindError.NOT_FOUND);
+      return new ServiceError("Not found", EServiceKindError.NOT_FOUND);
     }
 
     return new UserInfo(
@@ -89,27 +89,6 @@ export class GithubApiService extends GithubRepository {
     );
   }
 
-  private handleError(responseErrors: GithubError[]): ServiceError {
-    const errors = responseErrors ?? [];
-
-    const isRateLimitExceeded = errors.some((error) =>
-      error.type.includes(EServiceKindError.RATE_LIMIT) ||
-      error.message.includes("rate limit")
-    );
-
-    if (isRateLimitExceeded) {
-      throw new ServiceError(
-        "Rate limit exceeded",
-        EServiceKindError.RATE_LIMIT,
-      );
-    }
-
-    throw new ServiceError(
-      "unknown error",
-      EServiceKindError.NOT_FOUND,
-    );
-  }
-
   async executeQuery<T = unknown>(
     query: string,
     variables: { [key: string]: string },
@@ -120,36 +99,25 @@ export class GithubApiService extends GithubRepository {
         CONSTANTS.DEFAULT_GITHUB_RETRY_DELAY,
       );
       const response = await retry.fetch<Promise<T>>(async ({ attempt }) => {
-        const res = await soxa.post("", {}, {
-          data: { query: query, variables },
-          headers: {
-            Authorization: `bearer ${TOKENS[attempt]}`,
-          },
-        });
-        if (res?.data?.errors) {
-          return this.handleError(res?.data?.errors);
-        }
-        return res;
-      }) as QueryDefaultResponse<{ user: T }>;
+        return await requestGithubData(
+          query,
+          variables,
+          TOKENS[attempt],
+        );
+      });
 
-      return response?.data?.data?.user ??
-        new ServiceError("not found", EServiceKindError.NOT_FOUND);
+      return response;
     } catch (error) {
-      if (error instanceof ServiceError) {
-        Logger.error(error);
-        return error;
+      if (error.cause instanceof ServiceError) {
+        Logger.error(error.cause.message);
+        return error.cause;
       }
-      // TODO: Move this to a logger instance later
       if (error instanceof Error && error.cause) {
         Logger.error(JSON.stringify(error.cause, null, 2));
       } else {
         Logger.error(error);
       }
-
-      return new ServiceError(
-        "Rate limit exceeded",
-        EServiceKindError.RATE_LIMIT,
-      );
+      return new ServiceError("not found", EServiceKindError.NOT_FOUND);
     }
   }
 }

src/Services/__mocks__/notFoundUserMock.json

@@ -0,0 +1,20 @@
+{
+  "data": {
+    "user": null
+  },
+  "errors": [
+    {
+      "type": "NOT_FOUND",
+      "path": [
+        "user"
+      ],
+      "locations": [
+        {
+          "line": 2,
+          "column": 5
+        }
+      ],
+      "message": "Could not resolve to a User with the login of 'alekinho'."
+    }
+  ]
+}

src/Services/__mocks__/rateLimitMock.json

@@ -0,0 +1,18 @@
+{
+  "exceeded": {
+    "data": {
+      "documentation_url": "https://docs.github.com/en/free-pro-team@latest/rest/overview/resources-in-the-rest-api#secondary-rate-limits",
+      "message": "You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID DBD8:FB98:31801A8:3222432:65195FDB."
+    }
+  },
+  "rate_limit": {
+    "data": {
+      "errors": [
+        {
+          "type": "RATE_LIMITED",
+          "message": "API rate limit exceeded for user ID 10711649."
+        }
+      ]
+    }
+  }
+}