Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion sdk/dart/lib/src/base_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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}';
Expand Down
16 changes: 16 additions & 0 deletions sdk/dart/test/client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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: '<html>Bad Gateway</html>',
contentType: 'text/html',
);
try {
await m.client.links.list();
fail('expected ShrtnrError');
} on ShrtnrError catch (e) {
expect(e.status, 200);
}
});
});

// ---- 3. links.get ----
Expand Down
8 changes: 7 additions & 1 deletion sdk/typescript/src/internal/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
11 changes: 11 additions & 0 deletions sdk/typescript/tests/client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, "<html>Bad Gateway</html>", "text/html");
try {
await client().links.list();
expect.unreachable();
} catch (e) {
expect(e).toBeInstanceOf(ShrtnrError);
expect((e as ShrtnrError).status).toBe(200);
}
});
});

// ============================================================
Expand Down
19 changes: 19 additions & 0 deletions src/__tests__/handler/api-links.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
});
});
34 changes: 34 additions & 0 deletions src/__tests__/handler/mcp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
14 changes: 13 additions & 1 deletion src/__tests__/service/bundle-service.test.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -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", () => {
Expand Down
25 changes: 24 additions & 1 deletion src/__tests__/service/link-service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
});
});
2 changes: 1 addition & 1 deletion src/api/qr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ export class ShrtnrMCP extends McpAgent<Env, Record<string, never>, 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");
Expand Down
3 changes: 2 additions & 1 deletion src/services/bundle-management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 3 additions & 1 deletion src/services/link-management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down