diff --git a/sdk/dart/lib/src/base_client.dart b/sdk/dart/lib/src/base_client.dart index b93ef92..6696078 100644 --- a/sdk/dart/lib/src/base_client.dart +++ b/sdk/dart/lib/src/base_client.dart @@ -72,7 +72,11 @@ class ShrtnrBaseClient { if (response.statusCode >= 200 && response.statusCode < 300) { if (response.statusCode == 204 || response.body.isEmpty) return null; - return jsonDecode(response.body); + try { + return jsonDecode(response.body); + } catch (e) { + throw ShrtnrError(response.statusCode, 'Invalid JSON response: $e'); + } } String serverMessage = 'HTTP ${response.statusCode}'; diff --git a/sdk/dart/test/client_test.dart b/sdk/dart/test/client_test.dart index f9f4db2..71e4de2 100644 --- a/sdk/dart/test/client_test.dart +++ b/sdk/dart/test/client_test.dart @@ -227,6 +227,22 @@ void main() { expect(e.serverMessage, contains('connection refused')); } }); + + test( + 'wraps a non-JSON 2xx body in ShrtnrError instead of throwing a raw FormatException', + () async { + final m = _mock( + status: 200, + body: 'Bad Gateway', + contentType: 'text/html', + ); + try { + await m.client.links.list(); + fail('expected ShrtnrError'); + } on ShrtnrError catch (e) { + expect(e.status, 200); + } + }); }); // ---- 3. links.get ---- diff --git a/sdk/typescript/src/internal/http.ts b/sdk/typescript/src/internal/http.ts index 902021b..53aa420 100644 --- a/sdk/typescript/src/internal/http.ts +++ b/sdk/typescript/src/internal/http.ts @@ -59,7 +59,13 @@ export class HttpClient { if (res.status === 204) return undefined as T; - const json: unknown = await res.json(); + let json: unknown; + try { + json = await res.json(); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new ShrtnrError(res.status, `Invalid JSON response: ${msg}`); + } return keysToCamel(json) as T; } diff --git a/sdk/typescript/tests/client.test.ts b/sdk/typescript/tests/client.test.ts index 09581b2..6c01d6c 100644 --- a/sdk/typescript/tests/client.test.ts +++ b/sdk/typescript/tests/client.test.ts @@ -109,6 +109,17 @@ describe("Error handling", () => { expect((e as ShrtnrError).status).toBe(0); } }); + + it("wraps a non-JSON 2xx body in ShrtnrError instead of throwing a raw SyntaxError", async () => { + mockFetch(200, "Bad Gateway", "text/html"); + try { + await client().links.list(); + expect.unreachable(); + } catch (e) { + expect(e).toBeInstanceOf(ShrtnrError); + expect((e as ShrtnrError).status).toBe(200); + } + }); }); // ============================================================ diff --git a/src/__tests__/handler/api-links.test.ts b/src/__tests__/handler/api-links.test.ts index 559af3f..82515c8 100644 --- a/src/__tests__/handler/api-links.test.ts +++ b/src/__tests__/handler/api-links.test.ts @@ -1287,4 +1287,23 @@ describe("GET /_/api/links/:id/qr size validation", () => { expect(explicitPrimary.status).toBe(200); expect(await noSlug.text()).toBe(await explicitPrimary.text()); }); + + it("matches an explicit slug regardless of case, since stored slugs are always lowercase", async () => { + const createRes = await SELF.fetch( + authed("/_/admin/api/links", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ url: "https://example.com/qr-case" }), + }) + ); + expect(createRes.status).toBe(201); + const created = await createRes.json() as { id: number; slugs: { slug: string }[] }; + const slug = created.slugs[0].slug; + + const lower = await SELF.fetch(authed(`/_/admin/api/links/${created.id}/qr?slug=${slug}`)); + const upper = await SELF.fetch(authed(`/_/admin/api/links/${created.id}/qr?slug=${slug.toUpperCase()}`)); + expect(lower.status).toBe(200); + expect(upper.status).toBe(200); + expect(await upper.text()).toBe(await lower.text()); + }); }); diff --git a/src/__tests__/handler/mcp.test.ts b/src/__tests__/handler/mcp.test.ts index 18fd9ff..62a843d 100644 --- a/src/__tests__/handler/mcp.test.ts +++ b/src/__tests__/handler/mcp.test.ts @@ -527,6 +527,40 @@ describe("MCP get_link_qr", () => { // (api/qr.ts), so redirect.ts records the scan with link_mode = "qr". expect(text).toContain(`https://shrtnr.test/${slug}?utm_medium=qr`); }); + + it("matches an explicit slug argument regardless of case, since stored slugs are always lowercase", async () => { + const created = await createLink(env as any, { url: "https://example.com/qr-case" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + const slug = created.data.slugs[0].slug; + + const sessionId = await initSession(); + const res = await SELF.fetch( + new Request("https://shrtnr.test/_/mcp", { + method: "POST", + headers: { + "Content-Type": "application/json", + Accept: "application/json, text/event-stream", + "mcp-session-id": sessionId, + }, + body: JSON.stringify({ + jsonrpc: "2.0", + id: 5, + method: "tools/call", + params: { + name: "get_link_qr", + arguments: { link_id: created.data.id, slug: slug.toUpperCase(), base_url: "https://shrtnr.test" }, + }, + }), + }), + ); + expect(res.status).toBe(200); + const message = await readFirstSseMessage(res); + const result = message!.result as { isError?: boolean; content?: { type: string; text?: string }[] } | undefined; + expect(result?.isError).toBeFalsy(); + const text = result?.content?.find((c) => c.type === "text")?.text ?? ""; + expect(text).toContain(`https://shrtnr.test/${slug}?utm_medium=qr`); + }); }); describe("MCP error surface", () => { diff --git a/src/__tests__/service/bundle-service.test.ts b/src/__tests__/service/bundle-service.test.ts index bdd0885..712b7e6 100644 --- a/src/__tests__/service/bundle-service.test.ts +++ b/src/__tests__/service/bundle-service.test.ts @@ -1,7 +1,7 @@ import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; import { env } from "cloudflare:test"; import { applyMigrations, resetData } from "../setup"; -import { BundleRepository, LinkRepository } from "../../db"; +import { BundleRepository, ClickRepository, LinkRepository } from "../../db"; import * as svc from "../../services/bundle-management"; import type { Env } from "../../types"; @@ -321,6 +321,18 @@ describe("bundle concurrent-delete: null return propagates as 404", () => { expect(res.ok).toBe(false); if (!res.ok) expect(res.status).toBe(404); }); + + it("getBundleAnalytics returns 404 when getBundleStats returns null (concurrent deletion)", async () => { + const b = await BundleRepository.create(env.DB, { name: "Raced", createdBy: "a@b" }); + + // getBundleAnalytics's own getById check passes, but getBundleStats does + // an independent getById internally and can see the bundle gone by then. + const spy = vi.spyOn(ClickRepository, "getBundleStats").mockResolvedValueOnce(null); + const res = await svc.getBundleAnalytics(e, b.id, "30d", "a@b").finally(() => spy.mockRestore()); + + expect(res.ok).toBe(false); + if (!res.ok) expect(res.status).toBe(404); + }); }); describe("listBundlesForLink", () => { diff --git a/src/__tests__/service/link-service.test.ts b/src/__tests__/service/link-service.test.ts index dae3154..877aac5 100644 --- a/src/__tests__/service/link-service.test.ts +++ b/src/__tests__/service/link-service.test.ts @@ -6,9 +6,10 @@ import { createLink, getLink, getLinkBySlug, + setSlugPrimary, updateLink, } from "../../services/link-management"; -import { SettingRepository, SlugRepository } from "../../db"; +import { LinkRepository, SettingRepository, SlugRepository } from "../../db"; beforeAll(applyMigrations); beforeEach(resetData); @@ -440,3 +441,25 @@ describe("custom slug uniqueness under race conditions", () => { } }); }); + +describe("setSlugPrimary concurrent-delete: null return propagates as 404", () => { + it("returns 404 when the link is deleted between the membership check and the final read", async () => { + const created = await createLink(env as any, { url: "https://example.com/raced" }); + expect(created.ok).toBe(true); + if (!created.ok) return; + const slug = created.data.slugs[0].slug; + + // Simulate a concurrent delete landing after setSlugPrimary's initial + // membership check but before its final getById re-read. + const spy = vi.spyOn(LinkRepository, "getById") + .mockResolvedValueOnce(created.data) + .mockResolvedValueOnce(null); + try { + const result = await setSlugPrimary(env as any, created.data.id, slug); + expect(result.ok).toBe(false); + if (!result.ok) expect(result.status).toBe(404); + } finally { + spy.mockRestore(); + } + }); +}); diff --git a/src/api/qr.ts b/src/api/qr.ts index 15af509..6bfc822 100644 --- a/src/api/qr.ts +++ b/src/api/qr.ts @@ -30,7 +30,7 @@ export async function handleLinkQr(request: Request, env: Env, linkId: number): const link = result.data; const slug = requestedSlug - ? link.slugs.find((s) => s.slug === requestedSlug) + ? link.slugs.find((s) => s.slug === requestedSlug.toLowerCase()) : link.slugs.find((s) => s.is_primary) ?? link.slugs[0]; if (!slug) return json({ error: "Slug not found" }, 404); diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 63f7857..30a8048 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -479,7 +479,7 @@ export class ShrtnrMCP extends McpAgent, Props> { const link = result.data; const target = requestedSlug - ? link.slugs.find((s) => s.slug === requestedSlug) + ? link.slugs.find((s) => s.slug === requestedSlug.toLowerCase()) : (link.slugs.find((s) => s.is_primary) ?? link.slugs[0]); if (!target) return fail("Slug not found"); diff --git a/src/services/bundle-management.ts b/src/services/bundle-management.ts index 2bb0736..7d7b272 100644 --- a/src/services/bundle-management.ts +++ b/src/services/bundle-management.ts @@ -260,7 +260,8 @@ export async function getBundleAnalytics( const bundle = await BundleRepository.getById(env.DB, id); if (!bundle) return fail(404, "Bundle not found"); const stats = await ClickRepository.getBundleStats(env.DB, id, range, undefined, opts?.filters); - return ok(stats!); + if (!stats) return fail(404, "Bundle not found"); + return ok(stats); } export async function getBundleBreakdownPage( diff --git a/src/services/link-management.ts b/src/services/link-management.ts index 3641b5d..c35e803 100644 --- a/src/services/link-management.ts +++ b/src/services/link-management.ts @@ -298,7 +298,9 @@ export async function setSlugPrimary( if (!slugObj) return fail(404, "Slug not found on this link"); await SlugRepository.setPrimary(env.DB, linkId, slug); - return ok((await LinkRepository.getById(env.DB, linkId))!); + const updated = await LinkRepository.getById(env.DB, linkId); + if (!updated) return fail(404, "Link not found"); + return ok(updated); } export async function disableSlug(