From cf5b30eb12cf9be511b501fe0dc28ad668c282dd Mon Sep 17 00:00:00 2001 From: Matt Leon <108271225+wydengyre@users.noreply.github.com> Date: Sun, 25 Feb 2024 10:12:38 +0800 Subject: [PATCH] kill FetchWithError (#98) just use plain fetch type --- packages/cf/worker.test.ts | 2 +- packages/rai/feed.test.ts | 32 ++++++++--------- packages/rai/feed.ts | 7 ++-- packages/rai/fetch.ts | 51 ---------------------------- packages/rai/media.test.ts | 6 ++-- packages/rai/media.ts | 10 +++--- packages/server/feed-handler.test.ts | 24 ++++++------- packages/server/feed-handler.ts | 11 ++---- packages/server/handler.test.ts | 6 ++-- packages/server/handler.ts | 22 ++---------- 10 files changed, 49 insertions(+), 122 deletions(-) delete mode 100644 packages/rai/fetch.ts diff --git a/packages/cf/worker.test.ts b/packages/cf/worker.test.ts index 00679f7..b0082d7 100644 --- a/packages/cf/worker.test.ts +++ b/packages/cf/worker.test.ts @@ -89,7 +89,7 @@ async function rssFeedRai404() { const resp = await servers.worker.fetch("/programmi/404.xml"); assert(!resp.ok); - assert.strictEqual(resp.status, 404); + assert.strictEqual(resp.status, 500); } async function rssFeedRai500() { diff --git a/packages/rai/feed.test.ts b/packages/rai/feed.test.ts index e98af27..166e758 100644 --- a/packages/rai/feed.test.ts +++ b/packages/rai/feed.test.ts @@ -1,8 +1,7 @@ import { strict as assert } from "node:assert"; import test from "node:test"; -import { json } from "itty-router"; +import { error, json } from "itty-router"; import { RssConvertConf, feedToRss } from "./feed.js"; -import { FetchWithErr, NotOk, OkResponse } from "./fetch.js"; import feedJson from "./test/lastoriaingiallo.json" with { type: "json" }; import expectedJson from "./test/lastoriaingiallo.parsed.json" with { type: "json", @@ -23,8 +22,9 @@ test("feed", async (t) => { const raiBaseUrl = new URL("https://rai.dev/"); const poolSize = 5; // arbitrary -const mediaFetchFn: FetchWithErr = (input) => +const mediaFetchFn: typeof fetch = (input) => Promise.resolve({ + ok: true, url: input .toString() .replace(/.+cont=(.*)/, (_, cont) => `https://media.dev/${cont}.mp3`), @@ -33,27 +33,27 @@ const mediaFetchFn: FetchWithErr = (input) => "content-type": "audio/mpeg", "content-length": "123456789", }), - } as OkResponse); + } as Response); async function convertFeedSuccess() { - const fetchWithErr: FetchWithErr = async (input) => { + const fetch: typeof globalThis.fetch = async (input) => { return input.toString().endsWith("foo.json") - ? Promise.resolve(json(feedJson) as OkResponse) + ? Promise.resolve(json(feedJson)) : mediaFetchFn(input); }; - const conf: RssConvertConf = { raiBaseUrl, poolSize, fetchWithErr }; + const conf: RssConvertConf = { raiBaseUrl, poolSize, fetch }; const feed = await feedToRss(conf, "programmi/foo.json"); const parsed = parseFeed(feed); assert.deepStrictEqual(parsed, expectedJson); } async function convertFeed610Success() { - const fetchWithErr: FetchWithErr = async (input) => { + const fetch: typeof globalThis.fetch = async (input) => { return input.toString().endsWith("foo.json") - ? Promise.resolve(json(feedJson610) as OkResponse) + ? Promise.resolve(json(feedJson610)) : mediaFetchFn(input); }; - const conf: RssConvertConf = { raiBaseUrl, poolSize, fetchWithErr }; + const conf: RssConvertConf = { raiBaseUrl, poolSize, fetch }; const feed = await feedToRss(conf, "programmi/foo.json"); const parsed = parseFeed(feed); assert.deepStrictEqual(parsed, expectedJson610); @@ -61,16 +61,14 @@ async function convertFeed610Success() { async function convertFeed404() { const url = new URL("https://rai.dev/programmi/foo.json"); - const notFound = new NotOk(url, 404, "Not Found"); - const fetchWithErr = () => Promise.reject(notFound); - const conf: RssConvertConf = { raiBaseUrl, poolSize, fetchWithErr }; - await assert.rejects(feedToRss(conf, "programmi/foo.json"), notFound); + const fetch = () => Promise.resolve(error(404)); + const conf: RssConvertConf = { raiBaseUrl, poolSize, fetch }; + await assert.rejects(feedToRss(conf, "programmi/foo.json")); } async function convertFeedNonCompliantJson() { - const fetchWithErr = () => - Promise.resolve(json({ foo: "bar" }) as OkResponse); - const conf: RssConvertConf = { raiBaseUrl, poolSize, fetchWithErr }; + const fetch = () => Promise.resolve(json({ foo: "bar" })); + const conf: RssConvertConf = { raiBaseUrl, poolSize, fetch }; const expectedErr = /^Error: failed to parse feed JSON/; await assert.rejects(feedToRss(conf, "programmi/foo.json"), expectedErr); } diff --git a/packages/rai/feed.ts b/packages/rai/feed.ts index 3361731..02f7e15 100644 --- a/packages/rai/feed.ts +++ b/packages/rai/feed.ts @@ -1,6 +1,5 @@ import { PromisePool } from "@supercharge/promise-pool"; import { z } from "zod"; -import { FetchWithErr } from "./fetch.js"; import * as media from "./media.js"; export { RssConvertConf, feedToRss }; @@ -32,17 +31,17 @@ const schema = z.object({ type RssConvertConf = { raiBaseUrl: URL; poolSize: number; - fetchWithErr: FetchWithErr; + fetch: typeof fetch; }; async function feedToRss(c: RssConvertConf, relUrl: string): Promise { - const fetchInfo = media.mkFetchInfo(c.fetchWithErr); + const fetchInfo = media.mkFetchInfo(c.fetch); const convertor = new RssConvertor({ raiBaseUrl: c.raiBaseUrl, poolSize: c.poolSize, fetchInfo, }); const url = new URL(relUrl, c.raiBaseUrl); - const resp = await c.fetchWithErr(url); + const resp = await c.fetch(url); const json = await resp.json(); return convertor.convert(json); } diff --git a/packages/rai/fetch.ts b/packages/rai/fetch.ts deleted file mode 100644 index 8f1c3f9..0000000 --- a/packages/rai/fetch.ts +++ /dev/null @@ -1,51 +0,0 @@ -export { FetchWithErr, NotOk, OkResponse, mkFetchWithErr }; - -type FetchWithErr = ( - input: RequestInfo | URL, - init?: RequestInit, -) => Promise; - -type OkResponse = Response & { ok: true }; - -/** - * Returns a fetch function that errors on a non-ok response - * @param fetchFn - The underlying fetch function to use - * @returns A fetch function that throws an error for non-ok responses - */ -const mkFetchWithErr = - (fetchFn: typeof fetch): FetchWithErr => - async (input, init) => { - const res = await fetchFn(input, init); - if (!isOk(res)) { - // maybe we should use the url from the response? - const url = new URL(input.toString()); - throw new NotOk(url, res.status, res.statusText); - } - return res; - }; - -/** - * Error class for non-ok responses - */ -class NotOk extends Error { - readonly url: URL; - readonly status: number; - readonly statusText: string; - - /** - * @param url - The URL that resulted in a non-ok response - * @param status - The status code of the non-ok response - * @param statusText - The status text of the non-ok response - */ - constructor(url: URL, status: number, statusText: string) { - super(`Not ok: ${status} ${statusText} (${url})`); - this.name = "NotOkError"; - this.url = url; - this.status = status; - this.statusText = statusText; - } -} - -function isOk(res: Response): res is OkResponse { - return res.ok; -} diff --git a/packages/rai/media.test.ts b/packages/rai/media.test.ts index d92b2cb..21a49ba 100644 --- a/packages/rai/media.test.ts +++ b/packages/rai/media.test.ts @@ -1,6 +1,5 @@ import { strict as assert } from "node:assert"; import test from "node:test"; -import { FetchWithErr, OkResponse } from "./fetch.js"; import { mkFetchInfo } from "./media.js"; test("media", (t) => { @@ -11,15 +10,16 @@ async function fetchInfoSuccess() { const url = "https://mediapolisvod.rai.it/relinker/relinkerServlet.htm?cont=PE3wc6etKfssSlashNKfaoXssSlashpWcgeeqqEEqualeeqqEEqual"; const mediaUrl = new URL("https://test.dev/foo.mp3"); - const fetch: FetchWithErr = async () => + const fetch = async () => ({ + ok: true, url: mediaUrl.toString(), status: 200, headers: new Headers({ "content-type": "audio/mpeg", "content-length": "123456789", }), - }) as OkResponse; + }) as Response; const fetchInfo = mkFetchInfo(fetch); const info = await fetchInfo(url); diff --git a/packages/rai/media.ts b/packages/rai/media.ts index 49a911d..ca3668d 100644 --- a/packages/rai/media.ts +++ b/packages/rai/media.ts @@ -1,5 +1,3 @@ -import { FetchWithErr } from "./fetch.js"; - export { FetchInfo, MediaInfo, MediaUrl, mkFetchInfo }; type FetchInfo = (url: string) => Promise; @@ -15,7 +13,7 @@ type MediaInfo = { const relinkerRe = /^\?cont=[a-zA-Z0-9]+$/; const mkFetchInfo = - (fetchWithErr: FetchWithErr): FetchInfo => + (fetch: typeof globalThis.fetch): FetchInfo => async (url) => { const mediaUrl = mkMediaUrl(url); if (typeof mediaUrl === "string") { @@ -31,7 +29,11 @@ const mkFetchInfo = "User-Agent": chromeAgent, }, }; - const resp = await fetchWithErr(url, chromeHeadInit); + + const resp = await fetch(url, chromeHeadInit); + if (!resp.ok) { + throw new Error(`Failed to fetch: ${resp.status} ${resp.statusText}`); + } const contentLength = resp.headers.get("content-length"); const length = Number(contentLength); diff --git a/packages/server/feed-handler.test.ts b/packages/server/feed-handler.test.ts index 02dca74..33f7712 100644 --- a/packages/server/feed-handler.test.ts +++ b/packages/server/feed-handler.test.ts @@ -1,11 +1,10 @@ import { strict as assert } from "node:assert"; import test from "node:test"; -import { FetchWithErr, NotOk, OkResponse } from "@raiplayrss/rai/fetch.js"; import feedJson from "@raiplayrss/rai/test/lastoriaingiallo.json"; import expectedJson from "@raiplayrss/rai/test/lastoriaingiallo.parsed.json" with { type: "json", }; -import { json } from "itty-router"; +import { json, status } from "itty-router"; import { parseFeed } from "../rai/test/parse-feed.js"; import { feedHandler } from "./feed-handler.js"; import * as logger from "./logger.js"; @@ -21,22 +20,22 @@ test("feed-handler", async (t) => { const raiBaseUrl = new URL("https://rai.dev/"); const mediaBaseUrl = new URL("https://media.dev/"); -const confWithFetch = (fetchWithErr: FetchWithErr) => ({ +const confWithFetch = (fetch: typeof globalThis.fetch) => ({ raiBaseUrl, poolSize: 5, - fetchWithErr, + fetch, logger: logger.disabled, }); async function rssFeedSuccess() { const req = new Request("https://test.dev/programmi/lastoriaingiallo.xml"); - const fetchMock: FetchWithErr = async (input, init) => { + const fetchMock: typeof fetch = async (input) => { const requestUrlStr = input.toString(); const url = new URL(requestUrlStr); const { pathname, search } = url; if (pathname === "/programmi/lastoriaingiallo.json") { - return json(feedJson) as OkResponse; + return json(feedJson); } const relinkerRel = "/relinker/relinkerServlet.htm"; @@ -48,12 +47,13 @@ async function rssFeedSuccess() { ); const url = `${urlStart}.mp3`; return { + ok: true, url: url, headers: new Headers({ "Content-Type": "audio/mpeg", "Content-Length": "123456789", }), - } as OkResponse; + } as Response; } throw new Error(`unexpected request: ${requestUrlStr}`); @@ -71,22 +71,22 @@ async function rssFeedSuccess() { async function rssFeedFail404() { const url = new URL("https://test.dev/programmi/nonexistent.xml"); const req = new Request(url); - const fetchMock = () => Promise.reject(new NotOk(url, 404, "not found")); + const fetchMock = async () => status(404); const conf = confWithFetch(fetchMock); const resp = await feedHandler(conf, req); - assert.strictEqual(resp.status, 404); + assert.strictEqual(resp.status, 500); assert.strictEqual(resp.headers.get("Content-Type"), "application/xml"); const text = await resp.text(); assert.strictEqual( text, - "404not found", + "500server error", ); } async function rssFeedFail500() { const url = new URL("https://test.dev/programmi/500.xml"); const req = new Request(url); - const fetchMock = () => Promise.reject(new NotOk(url, 500, "server error")); + const fetchMock = async () => status(500); const conf = confWithFetch(fetchMock); const resp = await feedHandler(conf, req); assert.strictEqual(resp.status, 500); @@ -100,7 +100,7 @@ async function rssFeedFail500() { async function rssFeedFailNonCompliantJson() { const req = new Request("https://test.dev/programmi/corrupt.xml"); - const fetchMock = () => Promise.resolve(json({ foo: "bar" }) as OkResponse); + const fetchMock = async () => json({ foo: "bar" }); const conf = confWithFetch(fetchMock); const resp = await feedHandler(conf, req); diff --git a/packages/server/feed-handler.ts b/packages/server/feed-handler.ts index 75124e4..aba40e6 100644 --- a/packages/server/feed-handler.ts +++ b/packages/server/feed-handler.ts @@ -1,5 +1,4 @@ import { feedToRss } from "@raiplayrss/rai/feed.js"; -import { FetchWithErr, NotOk } from "@raiplayrss/rai/fetch.js"; import { createResponse } from "itty-router"; import { Logger } from "./logger.js"; @@ -8,7 +7,7 @@ export { Config, feedHandler }; type Config = { raiBaseUrl: URL; poolSize: number; - fetchWithErr: FetchWithErr; + fetch: typeof fetch; logger: Logger; }; async function feedHandler(conf: Config, request: Request): Promise { @@ -23,12 +22,8 @@ async function feedHandler(conf: Config, request: Request): Promise { conf.logger.error("error converting feed", jsonPath, e); const contentType = "application/xml"; const headers = new Headers({ "Content-Type": contentType }); - let status = 500; - let body = "500server error"; - if (e instanceof NotOk && e.status === 404) { - status = e.status; - body = "404not found"; - } + const status = 500; + const body = `${status}server error`; return new Response(body, { status, headers }); } const rss = createResponse("application/rss+xml"); diff --git a/packages/server/handler.test.ts b/packages/server/handler.test.ts index 4a04242..3462c21 100644 --- a/packages/server/handler.test.ts +++ b/packages/server/handler.test.ts @@ -66,15 +66,15 @@ async function rssFeedSuccess() { async function rssFeedFail404() { const req = new Request("arbitrary://arbitrary/programmi/nonexistent.xml"); - const fetchMock = () => Promise.resolve(error(404, "not found")); + const fetchMock = async () => error(404, "not found"); const fetchHandler = fetchHandlerWithMock(fetchMock); const resp = await fetchHandler(req); - assert.strictEqual(resp.status, 404); + assert.strictEqual(resp.status, 500); const text = await resp.text(); assert.strictEqual( text, - "404not found", + "500server error", ); } diff --git a/packages/server/handler.ts b/packages/server/handler.ts index 4574521..5525f55 100644 --- a/packages/server/handler.ts +++ b/packages/server/handler.ts @@ -1,28 +1,12 @@ -import { mkFetchWithErr } from "@raiplayrss/rai/fetch.js"; -import { Router, error, html } from "itty-router"; +import { Router, error } from "itty-router"; import { feedHandler } from "./feed-handler.js"; -import { Logger } from "./logger.js"; +import type { Config } from "./feed-handler.js"; export { Config, FetchHandler, mkFetchHandler }; -type Config = { - raiBaseUrl: URL; - poolSize: number; - fetch: typeof fetch; - logger: Logger; -}; - type FetchHandler = (req: Request) => Promise; function mkFetchHandler(conf: Config): FetchHandler { - const fetchWithErr = mkFetchWithErr(conf.fetch); - - const fetchFeedConf = { - raiBaseUrl: conf.raiBaseUrl, - poolSize: conf.poolSize, - fetchWithErr, - logger: conf.logger, - }; - const fetchFeed = (request: Request) => feedHandler(fetchFeedConf, request); + const fetchFeed = (request: Request) => feedHandler(conf, request); const router = Router() .get("/programmi/:feed.xml", fetchFeed)