malpercio.dev
WIP! A BB-style forum, on the ATmosphere!
We're still working... we'll be back soon when we have something to show off!
node
typescript
hono
htmx
atproto
refactor(web): address code review feedback on ATB-60 import/export
- Bind errors in all bare catch blocks; add isProgrammingError re-throw
in export JSON parse and parseBody catch paths
- Split uploaded.text() and JSON.parse into separate try blocks for
distinct error messages and log entries
- Add logger.error to extractAppviewError catch and parseBody catch
- Add 100 KB file size guard before reading uploaded file
- Slugify colorScheme in export filename to guard against unexpected values
- Fix route registration comment: 4-segment path is distinct from 3-segment
/:rkey — registration order does not matter
- Rewrite cssOverrides drop comment to focus on portability and CSS bleed
- Update FCIS annotation to reference project one-file-per-route-group convention
- Add safety comment on fontUrls cast (isHttpsUrl verifies typeof === "string")
- Add tests: non-404 AppView error → 500, fontUrls non-array, AppView POST
network failure; change mockFetch.mock.calls[N] to .at(-1)! with URL assertion
+53
-3
apps/web/src/routes/__tests__/admin-themes.test.tsx
+53
-3
apps/web/src/routes/__tests__/admin-themes.test.tsx
···
857
857
expect(disposition).toContain("my-fancy-theme");
858
858
expect(disposition).toContain("light");
859
859
});
860
+
861
+
it("returns 500 when AppView returns a non-404 error for the theme", async () => {
862
+
// Tests the else branch: non-ok AND non-404 status (e.g. 503) falls through
863
+
// to the null-theme 500 page, not the 404 page.
864
+
setupAuthenticatedSession([MANAGE_THEMES]);
865
+
mockFetch.mockResolvedValueOnce(mockResponse({ error: "Service unavailable" }, false, 503));
866
+
const routes = await loadThemeRoutes();
867
+
const res = await routes.request("/admin/themes/abc123/export", {
868
+
headers: { cookie: "atbb_session=token" },
869
+
});
870
+
expect(res.status).toBe(500);
871
+
const html = await res.text();
872
+
expect(html.toLowerCase()).toMatch(/unavailable|failed/);
873
+
});
860
874
});
861
875
862
876
describe("createAdminThemeRoutes — POST /admin/themes/import", () => {
···
1045
1059
expect(res.status).toBe(302);
1046
1060
expect(res.headers.get("location")).toBe("/admin/themes");
1047
1061
1048
-
const apiCall = mockFetch.mock.calls[2]; // third call: POST /api/admin/themes
1062
+
const apiCall = mockFetch.mock.calls.at(-1)!; // last call: POST /api/admin/themes
1063
+
expect(apiCall[0]).toContain("/api/admin/themes");
1049
1064
const body = JSON.parse(apiCall[1].body) as Record<string, unknown>;
1050
1065
expect((body.tokens as Record<string, string>)["color-bg"]).toBe("#000000");
1051
1066
expect((body.tokens as Record<string, unknown>)["unknown-custom-token"]).toBeUndefined();
···
1066
1081
headers: { cookie: "atbb_session=token" },
1067
1082
});
1068
1083
expect(res.status).toBe(302);
1069
-
const apiCall = mockFetch.mock.calls[2];
1084
+
const apiCall = mockFetch.mock.calls.at(-1)!;
1085
+
expect(apiCall[0]).toContain("/api/admin/themes");
1070
1086
const body = JSON.parse(apiCall[1].body) as Record<string, unknown>;
1071
1087
expect(body.cssOverrides).toBeUndefined();
1072
1088
});
···
1090
1106
expect(loc.toLowerCase()).toMatch(/https|font url/);
1091
1107
});
1092
1108
1109
+
it("redirects with error when fontUrls is a non-array value", async () => {
1110
+
setupAuthenticatedSession([MANAGE_THEMES]);
1111
+
const routes = await loadThemeRoutes();
1112
+
const res = await routes.request("/admin/themes/import", {
1113
+
method: "POST",
1114
+
body: makeImportFile({
1115
+
name: "Test",
1116
+
colorScheme: "light",
1117
+
tokens: {},
1118
+
fontUrls: "https://fonts.example.com/single-string-not-array",
1119
+
}),
1120
+
headers: { cookie: "atbb_session=token" },
1121
+
});
1122
+
expect(res.status).toBe(302);
1123
+
const loc = decodeURIComponent(res.headers.get("location") ?? "");
1124
+
expect(loc).toContain("error=");
1125
+
expect(loc.toLowerCase()).toMatch(/fonturl|array/);
1126
+
});
1127
+
1128
+
it("redirects with error when AppView POST fetch throws (network failure)", async () => {
1129
+
setupAuthenticatedSession([MANAGE_THEMES]);
1130
+
mockFetch.mockRejectedValueOnce(new Error("fetch failed"));
1131
+
const routes = await loadThemeRoutes();
1132
+
const res = await routes.request("/admin/themes/import", {
1133
+
method: "POST",
1134
+
body: makeImportFile(validImport),
1135
+
headers: { cookie: "atbb_session=token" },
1136
+
});
1137
+
expect(res.status).toBe(302);
1138
+
const loc = decodeURIComponent(res.headers.get("location") ?? "");
1139
+
expect(loc).toContain("error=");
1140
+
expect(loc.toLowerCase()).toMatch(/unavailable/);
1141
+
});
1142
+
1093
1143
it("calls POST /api/admin/themes and redirects to /admin/themes on success", async () => {
1094
1144
setupAuthenticatedSession([MANAGE_THEMES]);
1095
1145
mockFetch.mockResolvedValueOnce(mockResponse({ uri: "at://...", cid: "bafytest" }, true, 201));
···
1102
1152
expect(res.status).toBe(302);
1103
1153
expect(res.headers.get("location")).toBe("/admin/themes");
1104
1154
1105
-
const apiCall = mockFetch.mock.calls[2];
1155
+
const apiCall = mockFetch.mock.calls.at(-1)!;
1106
1156
expect(apiCall[0]).toBe("http://localhost:3000/api/admin/themes");
1107
1157
const body = JSON.parse(apiCall[1].body) as Record<string, unknown>;
1108
1158
expect(body.name).toBe("Imported Theme");
+46
-11
apps/web/src/routes/admin-themes.tsx
+46
-11
apps/web/src/routes/admin-themes.tsx
···
1
1
// pattern: Mixed (unavoidable)
2
2
// Reason: Hono route file — JSX components, pure helpers (slugifyName, isHttpsUrl,
3
-
// sanitizeTokenValue), and route handlers (I/O) must coexist in one file per the
4
-
// framework's factory-function pattern used across the web package.
3
+
// sanitizeTokenValue), and route handlers (I/O) must coexist per this project's
4
+
// one-file-per-route-group convention (see other admin-*.tsx route files).
5
5
import { Hono } from "hono";
6
6
import { BaseLayout } from "../layouts/base.js";
7
7
import { PageHeader, EmptyState } from "../components/index.js";
···
94
94
try {
95
95
const data = (await res.json()) as { error?: string };
96
96
return data.error ?? fallback;
97
-
} catch {
97
+
} catch (error) {
98
+
logger.error("Failed to parse AppView error response body", {
99
+
operation: "extractAppviewError",
100
+
status: res.status,
101
+
error: error instanceof Error ? error.message : String(error),
102
+
});
98
103
return fallback;
99
104
}
100
105
}
···
582
587
});
583
588
584
589
// ── GET /admin/themes/:rkey/export ────────────────────────────────────────
585
-
// Must be registered before /:rkey so Hono matches the literal "/export" suffix first.
590
+
// Distinct from /:rkey — the 4-segment path cannot match the 3-segment /:rkey route.
586
591
587
592
app.get("/admin/themes/:rkey/export", async (c) => {
588
593
const resolvedTheme = c.get("theme") ?? FALLBACK_THEME;
···
616
621
if (res.ok) {
617
622
try {
618
623
theme = (await res.json()) as AdminThemeEntry;
619
-
} catch {
624
+
} catch (error) {
625
+
if (isProgrammingError(error)) throw error;
620
626
logger.error("Failed to parse theme response for export", {
621
627
operation: "GET /admin/themes/:rkey/export",
622
628
themeRkey,
629
+
error: error instanceof Error ? error.message : String(error),
623
630
});
624
631
}
625
632
} else {
···
658
665
fontUrls: theme.fontUrls ?? [],
659
666
};
660
667
661
-
const filename = `${slugifyName(theme.name)}-${theme.colorScheme}.json`;
668
+
const filename = `${slugifyName(theme.name)}-${slugifyName(theme.colorScheme)}.json`;
662
669
c.header("Content-Type", "application/json");
663
670
c.header("Content-Disposition", `attachment; filename="${filename}"`);
664
671
return c.body(JSON.stringify(exportData, null, 2), 200);
···
981
988
rawBody = await c.req.parseBody();
982
989
} catch (error) {
983
990
if (isProgrammingError(error)) throw error;
991
+
logger.error("Failed to parse import form body", {
992
+
operation: "POST /admin/themes/import",
993
+
error: error instanceof Error ? error.message : String(error),
994
+
});
984
995
return c.redirect(
985
996
`/admin/themes?error=${encodeURIComponent("Invalid form submission.")}`,
986
997
302
···
995
1006
);
996
1007
}
997
1008
998
-
// Parse the uploaded JSON file
1009
+
const MAX_IMPORT_BYTES = 100 * 1024; // 100 KB
1010
+
if (uploaded.size > MAX_IMPORT_BYTES) {
1011
+
return c.redirect(
1012
+
`/admin/themes?error=${encodeURIComponent("Import failed: file exceeds the 100 KB size limit.")}`,
1013
+
302
1014
+
);
1015
+
}
1016
+
1017
+
// Read the file text and parse as JSON — two separate try blocks so encoding
1018
+
// failures and JSON syntax errors produce distinct log entries.
1019
+
let text: string;
1020
+
try {
1021
+
text = await uploaded.text();
1022
+
} catch (error) {
1023
+
if (isProgrammingError(error)) throw error;
1024
+
logger.error("Failed to read uploaded theme file", {
1025
+
operation: "POST /admin/themes/import",
1026
+
error: error instanceof Error ? error.message : String(error),
1027
+
});
1028
+
return c.redirect(
1029
+
`/admin/themes?error=${encodeURIComponent("Import failed: could not read the uploaded file.")}`,
1030
+
302
1031
+
);
1032
+
}
1033
+
999
1034
let parsed: unknown;
1000
1035
try {
1001
-
const text = await uploaded.text();
1002
1036
parsed = JSON.parse(text);
1003
1037
} catch {
1004
1038
return c.redirect(
···
1073
1107
);
1074
1108
}
1075
1109
}
1110
+
// Safe: every element has passed isHttpsUrl(), which verifies typeof === "string"
1076
1111
fontUrls = obj.fontUrls as string[];
1077
1112
}
1078
1113
1079
-
// cssOverrides is silently dropped — not safe to import until sanitization
1080
-
// is universally applied at write time (it already is via ATB-62, but drop
1081
-
// on import keeps the JSON schema clean and avoids accidental CSS bleed).
1114
+
// cssOverrides is silently dropped — keeps the shared JSON schema portable
1115
+
// (no forum-specific CSS bleeding in) and avoids importing structural overrides
1116
+
// that may not make sense in the target forum's layout.
1082
1117
1083
1118
let apiRes: Response;
1084
1119
try {