From 1ccd004c778fbcfa781a39b42e83626631704ade Mon Sep 17 00:00:00 2001 From: Rushil Perera Date: Fri, 7 Jun 2024 23:42:41 -0400 Subject: [PATCH] fix: fetchFromMultipleSources returns errorOccurred only if all sources fail --- .../episodes/getByAniListId/index.ts | 4 ++-- src/controllers/search/index.ts | 4 ++-- src/controllers/title/index.ts | 4 ++-- src/libs/fetchFromMultipleSources.spec.ts | 23 ++++++++----------- src/libs/fetchFromMultipleSources.ts | 11 ++++----- 5 files changed, 21 insertions(+), 25 deletions(-) diff --git a/src/controllers/episodes/getByAniListId/index.ts b/src/controllers/episodes/getByAniListId/index.ts index b301349..1542f21 100644 --- a/src/controllers/episodes/getByAniListId/index.ts +++ b/src/controllers/episodes/getByAniListId/index.ts @@ -49,7 +49,7 @@ const app = new OpenAPIHono(); app.openapi(route, async (c) => { const aniListId = Number(c.req.param("aniListId")); - const { result: episodes, errors } = await fetchFromMultipleSources([ + const { result: episodes, errorOccurred } = await fetchFromMultipleSources([ () => { const isAnifyEnabled = readEnvVariable(c.env, "ENABLE_ANIFY"); return getEpisodesFromAnify(isAnifyEnabled, aniListId); @@ -64,7 +64,7 @@ app.openapi(route, async (c) => { ), ]); - if (errors?.length > 0) { + if (errorOccurred) { return c.json(ErrorResponse, { status: 500 }); } diff --git a/src/controllers/search/index.ts b/src/controllers/search/index.ts index a602c53..4dd23a6 100644 --- a/src/controllers/search/index.ts +++ b/src/controllers/search/index.ts @@ -39,14 +39,14 @@ app.openapi(route, async (c) => { const page = Number(c.req.query("page") ?? 1); const limit = Number(c.req.query("limit") ?? 10); - const { result: response, errors } = await fetchFromMultipleSources([ + const { result: response, errorOccurred } = await fetchFromMultipleSources([ () => fetchSearchResultsFromAnilist(query, page, limit), () => fetchSearchResultsFromAmvstrm(query, page, limit), ]); if (!response) { return c.json({ - success: (errors ?? []).length === 0, + success: !errorOccurred, results: [], hasNextPage: false, }); diff --git a/src/controllers/title/index.ts b/src/controllers/title/index.ts index d3d56c6..332ca07 100644 --- a/src/controllers/title/index.ts +++ b/src/controllers/title/index.ts @@ -48,12 +48,12 @@ app.openapi(route, async (c) => { const aniListId = Number(c.req.query("id")); const aniListToken = c.req.header("X-AniList-Token"); - const { result: title, errors } = await fetchFromMultipleSources([ + const { result: title, errorOccurred } = await fetchFromMultipleSources([ () => fetchTitleFromAnilist(aniListId, aniListToken ?? undefined), () => fetchTitleFromAmvstrm(aniListId), ]); - if (errors?.length > 0) { + if (errorOccurred) { return c.json(ErrorResponse, { status: 500 }); } diff --git a/src/libs/fetchFromMultipleSources.spec.ts b/src/libs/fetchFromMultipleSources.spec.ts index 9cc6731..decaebc 100644 --- a/src/libs/fetchFromMultipleSources.spec.ts +++ b/src/libs/fetchFromMultipleSources.spec.ts @@ -21,8 +21,8 @@ describe("fetchFromMultipleSources", () => { expect(result).toEqual(2); }); - it("has promises with valid responses, contains no errors", async () => { - const { errors } = await fetchFromMultipleSources([ + it("has promises with valid responses, no error occurred", async () => { + const { errorOccurred } = await fetchFromMultipleSources([ () => Promise.resolve(undefined), () => Promise.resolve(null), () => Promise.reject(), @@ -30,28 +30,25 @@ describe("fetchFromMultipleSources", () => { () => Promise.resolve(3), ]); - expect(errors).toBeNull(); + expect(errorOccurred).toBeFalse(); }); - it("has promises with no valid responses, returns null", async () => { + it("has promises that all throw, returns null", async () => { const { result } = await fetchFromMultipleSources([ - () => Promise.resolve(null), () => Promise.reject("error"), - () => Promise.resolve(undefined), + () => Promise.reject(new Error("error")), ]); expect(result).toBeNull(); }); - it("has promises with no valid responses, contains error", async () => { - const { errors } = await fetchFromMultipleSources([ - () => Promise.resolve(null), + it("has promises that all throw, contains error", async () => { + const { errorOccurred } = await fetchFromMultipleSources([ () => Promise.reject("error"), () => Promise.reject(new Error("error")), - () => Promise.resolve(undefined), ]); - expect(errors).toEqual(["error", new Error("error")]); + expect(errorOccurred).toBeTrue(); }); it("has promises but cache has value, returns cached value", async () => { @@ -71,7 +68,7 @@ describe("fetchFromMultipleSources", () => { }); it("has promises but cache has value, contains no errors", async () => { - const { errors } = await fetchFromMultipleSources( + const { errorOccurred } = await fetchFromMultipleSources( [ () => Promise.resolve(null), () => Promise.reject("error"), @@ -83,7 +80,7 @@ describe("fetchFromMultipleSources", () => { }, ); - expect(errors).toBeNull(); + expect(errorOccurred).toBeFalse(); }); it("has promises, no cached value, no valid response, should not save in cache", async () => { diff --git a/src/libs/fetchFromMultipleSources.ts b/src/libs/fetchFromMultipleSources.ts index f7f7622..3b498bf 100644 --- a/src/libs/fetchFromMultipleSources.ts +++ b/src/libs/fetchFromMultipleSources.ts @@ -10,7 +10,7 @@ type OptionalArgs = interface FetchFromMultipleSourcesResult { result: T | null; - errors: (Error | string | undefined)[] | null; + errorOccurred: boolean; } export async function fetchFromMultipleSources( @@ -19,21 +19,21 @@ export async function fetchFromMultipleSources( ): Promise> { let result = await args?.fetchFromCache?.(); if (result) { - return { result, errors: null }; + return { result, errorOccurred: false }; } if (fetchPromises.length === 0) { throw new Error("fetchPromises cannot be empty"); } - let errors: Record = {}; + let errorCount = 0; for (let i = 0; i < fetchPromises.length; i++) { const promise = fetchPromises[i]; try { result = await promise(); if (result) break; } catch (e) { - errors[i] = e as Error; + errorCount++; } } @@ -44,7 +44,6 @@ export async function fetchFromMultipleSources( result = result ?? null; return { result, - errors: - !result && Object.keys(errors).length > 0 ? Object.values(errors) : null, + errorOccurred: errorCount === fetchPromises.length, }; }