From ae000d743ef54368d21596d590e1acc0d90b370e Mon Sep 17 00:00:00 2001 From: Joe Mou Date: Thu, 3 Sep 2026 23:08:08 -0400 Subject: Handle route errors in middleware Every page route wrapped its scrape in tryRender, which awaited the promise, rendered the template on success, and turned a failure into a redirect or an error page. Hono's onError does that mapping for all of them at once, so handlers now await their scrape inline and both wrappers are gone. Fragments under /api/ still fail as an empty body rather than a page; onError checks for them first, since a fragment turns a redirect into a 404 instead of following it. RedirectError becomes an abstract base with InternalRedirectError and ExternalRedirectError under it, so the handler dispatches on the type instead of sniffing the location for an https:// prefix. One behavior change: a thrown value that is not an Error no longer renders error.eta, because Hono only routes Error instances to onError. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QERwoRJhuqMhKzzCUW4Skv --- src/app.ts | 104 +++++++++++++++++++++------------------------------- src/scraper.test.ts | 13 ++++--- src/scraper.ts | 12 ++++-- 3 files changed, 56 insertions(+), 73 deletions(-) (limited to 'src') diff --git a/src/app.ts b/src/app.ts index bbe07fc..2fa5832 100644 --- a/src/app.ts +++ b/src/app.ts @@ -18,7 +18,9 @@ import { getGitHubTree, getGitHubWiki, getGitHubWikiPages, + ExternalRedirectError, GitHubHTTPError, + InternalRedirectError, RedirectError, } from "./scraper.ts"; @@ -64,73 +66,53 @@ export function createApp(eta: Eta) { return redirectTo(c, githubUrlFor(c)); } - app.get("/", async (c) => { - return c.render("home.eta"); - }); - - async function tryRender(c: Context, template: string, promise: Promise) { - let data: T; - try { - data = await promise; - } catch (e) { - if (e instanceof RedirectError) { - if (e.location.startsWith("https://")) { - return redirectTo(c, e.location); - } - return c.redirect(e.location); - } else if (e instanceof GitHubHTTPError) { - c.status(e.status as StatusCode); - const message = `GitHub responded with HTTP ${e.status} ${e.message}`; - return c.render("error.eta", { title: e.message, message }); - } else { - c.status(500); - return c.render("error.eta", { message: "" + e }); - } - } - return c.render(template, data); - } - - // For fragments fetched asynchronously by client-side JS (see public/static/refs.js): - // errors render as an empty body rather than the full error.eta page. - async function tryRenderFragment( - c: Context, - template: string, - promise: Promise, - ) { - let data: T; - try { - data = await promise; - } catch (e) { - const status = + app.onError((e, c) => { + // Fragments fetched asynchronously by client-side JS (see public/static/refs.js) + // fail as an empty body rather than the full error.eta page. + if (c.req.path.startsWith("/api/")) { + c.status( e instanceof GitHubHTTPError ? (e.status as StatusCode) : e instanceof RedirectError ? 404 - : 500; - c.status(status); + : 500, + ); return c.body(null); } - return c.render(template, data); - } + + if (e instanceof InternalRedirectError) return c.redirect(e.location); + if (e instanceof ExternalRedirectError) return redirectTo(c, e.location); + if (e instanceof GitHubHTTPError) { + c.status(e.status as StatusCode); + const message = `GitHub responded with HTTP ${e.status} ${e.message}`; + return c.render("error.eta", { title: e.message, message }); + } + c.status(500); + return c.render("error.eta", { message: "" + e }); + }); + + app.get("/", async (c) => { + return c.render("home.eta"); + }); app.get("/:owner", async (c) => { const { owner } = c.req.param(); - return tryRender(c, "owner.eta", getGitHubOwner(owner)); + return c.render("owner.eta", await getGitHubOwner(owner)); }); app.get("/:owner/:repo", async (c) => { const { owner, repo } = c.req.param(); - return tryRender(c, "repo.eta", getGitHubRepo(owner, repo)); + return c.render("repo.eta", await getGitHubRepo(owner, repo)); }); app.get("/:owner/:repo/tree/:branch/:path{.*}?", async (c) => { const { owner, repo, branch, path = "" } = c.req.param(); - return tryRender(c, "tree.eta", getGitHubTree(owner, repo, branch, path)); + return c.render("tree.eta", await getGitHubTree(owner, repo, branch, path)); }); app.get("/:owner/:repo/blob/:branch/:path{.*}", async (c) => { const { owner, repo, branch, path } = c.req.param(); - return tryRender(c, "blob.eta", getGitHubBlob(owner, repo, branch, path)); + return c.render("blob.eta", await getGitHubBlob(owner, repo, branch, path)); }); app.get("/:owner/:repo/raw/:branch/:path{.*}", async (c) => { @@ -168,12 +150,12 @@ export function createApp(eta: Eta) { // page can never be mistaken for one. app.get("/:owner/:repo/wiki", async (c) => { const { owner, repo } = c.req.param(); - return tryRender(c, "wiki.eta", getGitHubWiki(owner, repo)); + return c.render("wiki.eta", await getGitHubWiki(owner, repo)); }); app.get("/:owner/:repo/wiki/_pages", async (c) => { const { owner, repo } = c.req.param(); - return tryRender(c, "wikiPages.eta", getGitHubWikiPages(owner, repo)); + return c.render("wikiPages.eta", await getGitHubWikiPages(owner, repo)); }); // An older revision of a page renders exactly like its current one. @@ -182,7 +164,7 @@ export function createApp(eta: Eta) { if (page.startsWith("_") || !/^[0-9a-f]{40}$/.test(oid)) { return redirectToGitHub(c); } - return tryRender(c, "wiki.eta", getGitHubWiki(owner, repo, page, oid)); + return c.render("wiki.eta", await getGitHubWiki(owner, repo, page, oid)); }); app.get("/:owner/:repo/wiki/:page", async (c) => { @@ -190,7 +172,7 @@ export function createApp(eta: Eta) { if (page.startsWith("_")) { return redirectToGitHub(c); } - return tryRender(c, "wiki.eta", getGitHubWiki(owner, repo, page)); + return c.render("wiki.eta", await getGitHubWiki(owner, repo, page)); }); app.get("/:owner/:repo/issues", async (c) => { @@ -203,13 +185,13 @@ export function createApp(eta: Eta) { return c.redirect(`/${owner}/${repo}/issues?q=${encodeURIComponent(`is:issue ${q}`)}`); } - return tryRender(c, "issues.eta", getGitHubIssues(owner, repo, { q })); + return c.render("issues.eta", await getGitHubIssues(owner, repo, { q })); }); app.get("/:owner/:repo/pulls", async (c) => { const { owner, repo } = c.req.param(); const q = c.req.query("q"); - return tryRender(c, "pulls.eta", getGitHubPulls(owner, repo, { q })); + return c.render("pulls.eta", await getGitHubPulls(owner, repo, { q })); }); app.get("/:owner/:repo/search", async (c) => { @@ -229,7 +211,7 @@ export function createApp(eta: Eta) { app.get("/:owner/:repo/commits/:branch/:path{.*}?", async (c) => { const { owner, repo, branch, path = "" } = c.req.param(); - return tryRender(c, "commits.eta", getGitHubCommits(owner, repo, branch, path)); + return c.render("commits.eta", await getGitHubCommits(owner, repo, branch, path)); }); // Internal endpoints fetched asynchronously by client-side JS, not meant to @@ -246,32 +228,28 @@ export function createApp(eta: Eta) { if (view !== "tree" && view !== "blob" && view !== "commits") { return c.notFound(); } - const refs = getGitHubRefs(owner, repo, type).then((data) => ({ ...data, view, path })); - return tryRenderFragment(c, "_refs.eta", refs); + const refs = await getGitHubRefs(owner, repo, type); + return c.render("_refs.eta", { ...refs, view, path }); }); app.get("/api/:owner/:repo/latest-commit/:branch/:path{.*}?", async (c) => { const { owner, repo, branch, path = "" } = c.req.param(); - return tryRenderFragment( - c, - "_latestCommit.eta", - getGitHubLatestCommit(owner, repo, branch, path), - ); + return c.render("_latestCommit.eta", await getGitHubLatestCommit(owner, repo, branch, path)); }); app.get("/api/:owner/:repo/sidebar", async (c) => { const { owner, repo } = c.req.param(); - return tryRenderFragment(c, "_sidebar.eta", getGitHubSidebar(owner, repo)); + return c.render("_sidebar.eta", await getGitHubSidebar(owner, repo)); }); app.get("/:owner/:repo/releases/tag/:tag", async (c) => { const { owner, repo, tag } = c.req.param(); - return tryRender(c, "release.eta", getGitHubRelease(owner, repo, tag)); + return c.render("release.eta", await getGitHubRelease(owner, repo, tag)); }); app.get("/:owner/:repo/releases", async (c) => { const { owner, repo } = c.req.param(); - return tryRender(c, "releases.eta", getGitHubReleases(owner, repo)); + return c.render("releases.eta", await getGitHubReleases(owner, repo)); }); app.all("*", async (c) => { diff --git a/src/scraper.test.ts b/src/scraper.test.ts index f6be2b6..8dcb0b8 100644 --- a/src/scraper.test.ts +++ b/src/scraper.test.ts @@ -16,8 +16,9 @@ import { getGitHubTree, getGitHubWiki, getGitHubWikiPages, + ExternalRedirectError, GitHubHTTPError, - RedirectError, + InternalRedirectError, } from "./scraper.ts"; describe("GitHub scraper", () => { @@ -215,7 +216,7 @@ describe("GitHub scraper", () => { const oid = "b39c421b98f49d83ae50ec502c3ddfc3bf28f2c6"; await assert.rejects(getGitHubTree("actions", "deploy-pages", oid, "src/index.js"), (err) => { - assert(err instanceof RedirectError, "error should be a RedirectError"); + assert(err instanceof InternalRedirectError, "error should be an InternalRedirectError"); assert.strictEqual(err.location, `/actions/deploy-pages/blob/${oid}/src/index.js`); return true; }); @@ -490,7 +491,7 @@ describe("GitHub scraper", () => { it("should redirect to the repository when it has no wiki", async () => { await assert.rejects(getGitHubWiki("git", "git"), (err) => { - assert(err instanceof RedirectError, "error should be a RedirectError"); + assert(err instanceof InternalRedirectError, "error should be an InternalRedirectError"); assert.strictEqual(err.location, "/git/git"); return true; }); @@ -571,7 +572,7 @@ describe("GitHub scraper", () => { it("should redirect to GitHub for non-profile pages", async () => { await assert.rejects(getGitHubOwner("features"), (err) => { - assert(err instanceof RedirectError, "error should be a RedirectError"); + assert(err instanceof ExternalRedirectError, "error should be an ExternalRedirectError"); assert.strictEqual(err.location, "https://github.com/features"); return true; }); @@ -599,7 +600,7 @@ describe("GitHub scraper", () => { await assert.rejects( getGitHubBlob("earendil-works", "pi", "main", "packages/agent"), (err) => { - assert(err instanceof RedirectError, "error should be a RedirectError"); + assert(err instanceof InternalRedirectError, "error should be an InternalRedirectError"); assert.strictEqual(err.location, "/earendil-works/pi/tree/main/packages/agent"); return true; }, @@ -610,7 +611,7 @@ describe("GitHub scraper", () => { await assert.rejects( getGitHubTree("earendil-works", "pi", "main", "packages/agent/package.json"), (err) => { - assert(err instanceof RedirectError, "error should be a RedirectError"); + assert(err instanceof InternalRedirectError, "error should be an InternalRedirectError"); assert.strictEqual( err.location, "/earendil-works/pi/blob/main/packages/agent/package.json", diff --git a/src/scraper.ts b/src/scraper.ts index 6461b19..fb48700 100644 --- a/src/scraper.ts +++ b/src/scraper.ts @@ -13,16 +13,20 @@ export class GitHubHTTPError extends Error { } } -export class RedirectError extends Error { +export abstract class RedirectError extends Error { location: string; constructor(location: string) { super(`Redirect to ${location}`); - this.name = "RedirectError"; + this.name = this.constructor.name; this.location = location; } } +export class InternalRedirectError extends RedirectError {} + +export class ExternalRedirectError extends RedirectError {} + interface TreeItem { contentType: "directory" | "file"; name: string; @@ -423,7 +427,7 @@ async function fetchGitHubResponse(path: string, accept: string): Promise { return parseOrganization(document, owner); } // Not a profile page (marketing pages like github.com/features, etc.). - throw new RedirectError(`https://github.com/${owner}`); + throw new ExternalRedirectError(`https://github.com/${owner}`); } // The text directly inside an element, ignoring any nested elements' text. -- cgit v1.3.1