Skip to content

Commit 3fbceb2

Browse files
Brooooooklynclaude
andauthored
fix(api): retry CF asset session on plain-text response and dedupe identical deploys (#37)
* fix(api): retry CF asset session on plain-text response and dedupe identical deploys CF's WfP assets-upload-session API intermittently returns plain text ("Assets have already been uploaded") instead of JSON when all asset hashes already exist. This varies by edge node and causes deploys to fail with "Unexpected token 'A'" JSON parse errors. Two fixes: 1. Retry: createAssetUploadSession now retries up to 3 times with backoff (1s, 2s, 3s) when CF returns plain text. HTTP errors still fail immediately. 2. Dedupe: Before creating a deployment record, the deploy handler compares manifest + asset hashes + secrets state with the current live deployment. If identical, returns immediately without any CF API calls. This handles the common case of CI redeploying unchanged code (e.g. same commit daily). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(api): move dedupe before quota check, drop secrets comparison Address two review comments: 1. Move the identical-deploy fast path before the daily quota check so users at their deploy limit still get instant skips for unchanged deploys instead of 429. 2. Remove hasSecretsChangedSince from the dedupe gate. The secrets API already live-pushes values to the active worker via putWorkerSecret. If that push fails the user gets a 502 and should retry the secret PUT, not redeploy. Keeping the check caused unnecessary full deploys after every secret rotation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent efba2fb commit 3fbceb2

4 files changed

Lines changed: 305 additions & 44 deletions

File tree

‎packages/api/src/assets.ts‎

Lines changed: 36 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -26,41 +26,57 @@ function tryParseJson<T>(text: string): T | null {
2626
}
2727
}
2828

29+
const ASSET_SESSION_MAX_RETRIES = 3;
30+
const ASSET_SESSION_RETRY_DELAY_MS = 1000;
31+
2932
/**
3033
* Step 1: Create an asset upload session with CF.
3134
* Sends the asset manifest (path → {hash, size}) and receives a completion JWT
3235
* plus "buckets" — groups of hashes that CF needs uploaded.
36+
*
37+
* CF's WfP assets API sometimes returns plain text (e.g. "Assets have already
38+
* been uploaded") instead of JSON when all asset hashes already exist. This is
39+
* a CF bug — the response varies by edge node. Retrying typically hits a
40+
* different node that returns proper JSON.
3341
*/
3442
export async function createAssetUploadSession(
3543
accountId: string,
3644
apiToken: string,
3745
namespace: string,
3846
workerName: string,
3947
manifest: Record<string, AssetManifestEntry>,
48+
{ retryDelayMs = ASSET_SESSION_RETRY_DELAY_MS }: { retryDelayMs?: number } = {},
4049
): Promise<{ jwt: string; buckets: string[][] }> {
41-
const response = await fetch(
42-
`https://api.cloudflare.com/client/v4/accounts/${accountId}/workers/dispatch/namespaces/${namespace}/scripts/${workerName}/assets-upload-session`,
43-
{
44-
method: "POST",
45-
headers: {
46-
Authorization: `Bearer ${apiToken}`,
47-
"Content-Type": "application/json",
48-
},
49-
body: JSON.stringify({ manifest }),
50-
},
51-
);
50+
const url = `https://api.cloudflare.com/client/v4/accounts/${accountId}/workers/dispatch/namespaces/${namespace}/scripts/${workerName}/assets-upload-session`;
51+
const headers = {
52+
Authorization: `Bearer ${apiToken}`,
53+
"Content-Type": "application/json",
54+
};
55+
const body = JSON.stringify({ manifest });
5256

53-
if (!response.ok) {
54-
const error = await response.text();
55-
throw new Error(`asset upload session failed: ${error}`);
56-
}
57+
let lastText = "";
58+
for (let attempt = 0; attempt <= ASSET_SESSION_MAX_RETRIES; attempt++) {
59+
if (attempt > 0) {
60+
await new Promise((r) => setTimeout(r, retryDelayMs * attempt));
61+
}
62+
63+
const response = await fetch(url, { method: "POST", headers, body });
64+
65+
if (!response.ok) {
66+
const error = await response.text();
67+
throw new Error(`asset upload session failed: ${error}`);
68+
}
5769

58-
const text = await response.text();
59-
const data = tryParseJson<UploadSessionResponse>(text);
60-
if (!data?.result?.jwt || !Array.isArray(data.result.buckets)) {
61-
throw new Error(`asset upload session returned invalid response: ${text}`);
70+
const text = await response.text();
71+
const data = tryParseJson<UploadSessionResponse>(text);
72+
if (data?.result?.jwt && Array.isArray(data.result.buckets)) {
73+
return { jwt: data.result.jwt, buckets: data.result.buckets };
74+
}
75+
76+
lastText = text;
6277
}
63-
return { jwt: data.result.jwt, buckets: data.result.buckets };
78+
79+
throw new Error(`asset upload session returned invalid response: ${lastText}`);
6480
}
6581

6682
/**

‎packages/api/src/routes/deploy.ts‎

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -220,15 +220,6 @@ app.post("/:id/deploy", async (c) => {
220220
await updateDeploymentStatus(c.get("db"), activeDeploy.id, "failed", "timed out");
221221
}
222222

223-
// Enforce deploy count limit
224-
const planLimits = getPlanLimits(user?.plan ?? "free");
225-
if (planLimits.deploysPerDay > 0) {
226-
const todayCount = await countUserDeploymentsToday(c.get("db"), c.get("userId"));
227-
if (!isWithinLimit(planLimits.deploysPerDay, todayCount)) {
228-
return c.json({ error: "deploy limit exceeded", limit: planLimits.deploysPerDay }, 429);
229-
}
230-
}
231-
232223
const formData = await c.req.formData();
233224
const manifestRaw = formData.get("manifest") as string | null;
234225
const incomingManifest: DeployManifest = manifestRaw
@@ -258,6 +249,61 @@ app.post("/:id/deploy", async (c) => {
258249
const manifest = enrichedManifestResult.manifest;
259250
const finalManifestRaw = JSON.stringify(manifest);
260251

252+
// Skip deploy if identical to the current live deployment (same manifest + same assets).
253+
// Secrets are not checked: the secrets API already live-pushes values to the active
254+
// worker via putWorkerSecret.
255+
if (currentLiveDeployment && currentLiveDeployment.status === "live") {
256+
const sameManifest = currentLiveDeployment.manifest === finalManifestRaw;
257+
const incomingAssetManifest = formData.get("assetManifest") as string | null;
258+
const sameAssets =
259+
incomingAssetManifest != null &&
260+
currentLiveDeployment.asset_hashes != null &&
261+
incomingAssetManifest === currentLiveDeployment.asset_hashes;
262+
if (sameManifest && sameAssets) {
263+
const siteDomain = c.env.SITE_DOMAIN ?? SITE_DOMAIN;
264+
const projectDomains = await getDomainsByProjectId(c.get("db"), projectId);
265+
const activeDomains = projectDomains.filter((d) => d.status === "active");
266+
const fullUrl =
267+
activeDomains.length > 0
268+
? `https://${activeDomains[0].hostname}`
269+
: `https://${project.slug}.${siteDomain}`;
270+
271+
const encoder = new TextEncoder();
272+
const stream = new ReadableStream({
273+
start(controller) {
274+
const emit = (event: DeployEvent) => {
275+
controller.enqueue(encoder.encode(JSON.stringify(event) + "\n"));
276+
};
277+
emit({ event: "start", deploymentId: currentLiveDeployment.id });
278+
emit({
279+
event: "done",
280+
deployment: currentLiveDeployment.id,
281+
url: fullUrl,
282+
urls: [
283+
...activeDomains.map((d) => `https://${d.hostname}`),
284+
`https://${project.slug}.${siteDomain}`,
285+
],
286+
assets: 0,
287+
workers: 0,
288+
});
289+
controller.close();
290+
},
291+
});
292+
return new Response(stream, {
293+
headers: { "Content-Type": "application/x-ndjson" },
294+
});
295+
}
296+
}
297+
298+
// Enforce deploy count limit
299+
const planLimits = getPlanLimits(user?.plan ?? "free");
300+
if (planLimits.deploysPerDay > 0) {
301+
const todayCount = await countUserDeploymentsToday(c.get("db"), c.get("userId"));
302+
if (!isWithinLimit(planLimits.deploysPerDay, todayCount)) {
303+
return c.json({ error: "deploy limit exceeded", limit: planLimits.deploysPerDay }, 429);
304+
}
305+
}
306+
261307
const source = (formData.get("source") as string | null) ?? null;
262308
const commit = (formData.get("commit") as string | null) ?? null;
263309

‎packages/api/test/integration/api-deploy.test.ts‎

Lines changed: 148 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -959,33 +959,167 @@ describe("POST /projects/:id/deploy", () => {
959959
expect(events.at(-1)).toMatchObject({ event: "done" });
960960
});
961961

962-
it("does not leak raw Cloudflare asset text into deploy error events", async () => {
962+
it(
963+
"sanitizes error after retries exhausted on persistent plain-text session response",
964+
{ timeout: 15_000 },
965+
async () => {
966+
const user = await createTestUser(env.db);
967+
const project = await createTestProject(env.db, user.id, "asset-text-error-test");
968+
mockCfApi({
969+
assetUploadSession: () => new Response("Assets have already been uploaded."),
970+
});
971+
972+
const res = await app.fetch(
973+
new Request(`http://localhost/projects/${project.id}/deploy`, {
974+
method: "POST",
975+
headers: { Authorization: `Bearer ${user.jwt}` },
976+
body: buildDeployForm({
977+
assets: { "index.html": "<h1>hi</h1>" },
978+
}),
979+
}),
980+
env,
981+
);
982+
983+
expect(res.status).toBe(200);
984+
const raw = await res.text();
985+
expect(raw).not.toContain("Assets have already been uploaded");
986+
const events = parseDeployStream(raw);
987+
expect(events.at(-1)).toMatchObject({
988+
event: "error",
989+
step: "asset_upload",
990+
message: "asset upload returned an unexpected Cloudflare response",
991+
});
992+
},
993+
);
994+
});
995+
996+
describe("Deploy deduplication (skip identical deploys)", () => {
997+
const manifest = { version: 3, bindings: {} };
998+
const assetManifest = { "/index.html": { hash: "abc12345678901234567890123456789", size: 100 } };
999+
1000+
it("skips deploy when manifest and assets are identical to live deployment", async () => {
9631001
const user = await createTestUser(env.db);
964-
const project = await createTestProject(env.db, user.id, "asset-text-error-test");
965-
mockCfApi({
966-
assetUploadSession: () => new Response("Assets have already been uploaded."),
1002+
const project = await createTestProject(env.db, user.id, "dedupe-app");
1003+
1004+
// Insert a live deployment with known manifest and asset_hashes
1005+
const liveDeployId = "dpl_live_dedupe";
1006+
await env.db.insert(deployments).values({
1007+
id: liveDeployId,
1008+
project_id: project.id,
1009+
status: "live",
1010+
manifest: JSON.stringify(manifest),
1011+
asset_hashes: JSON.stringify(assetManifest),
1012+
created_at: sql`now()`,
9671013
});
1014+
await env.db
1015+
.update(projects)
1016+
.set({ active_deployment_id: liveDeployId })
1017+
.where(eq(projects.id, project.id));
1018+
1019+
// Deploy with the same manifest and assetManifest
1020+
const form = buildDeployForm({ manifest, assets: { "index.html": "<h1>hi</h1>" } });
1021+
form.append("assetManifest", JSON.stringify(assetManifest));
1022+
form.append("preflight", "true");
9681023

1024+
// No CF API mock needed — deploy should be skipped entirely
9691025
const res = await app.fetch(
9701026
new Request(`http://localhost/projects/${project.id}/deploy`, {
9711027
method: "POST",
9721028
headers: { Authorization: `Bearer ${user.jwt}` },
973-
body: buildDeployForm({
974-
assets: { "index.html": "<h1>hi</h1>" },
975-
}),
1029+
body: form,
9761030
}),
9771031
env,
9781032
);
9791033

9801034
expect(res.status).toBe(200);
981-
const raw = await res.text();
982-
expect(raw).not.toContain("Assets have already been uploaded");
983-
const events = parseDeployStream(raw);
984-
expect(events.at(-1)).toMatchObject({
985-
event: "error",
986-
step: "asset_upload",
987-
message: "asset upload returned an unexpected Cloudflare response",
1035+
const events = parseDeployStream(await res.text());
1036+
expect(events).toHaveLength(2); // start + done only
1037+
expect(events[0]).toMatchObject({ event: "start", deploymentId: liveDeployId });
1038+
expect(events[1]).toMatchObject({ event: "done", deployment: liveDeployId });
1039+
});
1040+
1041+
it("skips deploy even when secrets changed (secrets are live-pushed via API)", async () => {
1042+
const user = await createTestUser(env.db);
1043+
const project = await createTestProject(env.db, user.id, "dedupe-secret-app");
1044+
1045+
// Insert a live deployment created in the past
1046+
const liveDeployId = "dpl_live_secret";
1047+
await env.db.insert(deployments).values({
1048+
id: liveDeployId,
1049+
project_id: project.id,
1050+
status: "live",
1051+
manifest: JSON.stringify(manifest),
1052+
asset_hashes: JSON.stringify(assetManifest),
1053+
created_at: sql`now() - interval '1 hour'`,
9881054
});
1055+
await env.db
1056+
.update(projects)
1057+
.set({ active_deployment_id: liveDeployId })
1058+
.where(eq(projects.id, project.id));
1059+
1060+
// Add a secret AFTER the deployment — secrets API already pushed it to the worker
1061+
await createTestProjectSecret(env.db, env, project.id, "API_KEY", "secret-value");
1062+
1063+
// Deploy with the same manifest — should still be skipped
1064+
const form = buildDeployForm({ manifest, assets: { "index.html": "<h1>hi</h1>" } });
1065+
form.append("assetManifest", JSON.stringify(assetManifest));
1066+
form.append("preflight", "true");
1067+
1068+
// No CF API mock needed — deploy should be skipped entirely
1069+
const res = await app.fetch(
1070+
new Request(`http://localhost/projects/${project.id}/deploy`, {
1071+
method: "POST",
1072+
headers: { Authorization: `Bearer ${user.jwt}` },
1073+
body: form,
1074+
}),
1075+
env,
1076+
);
1077+
1078+
expect(res.status).toBe(200);
1079+
const events = parseDeployStream(await res.text());
1080+
expect(events).toHaveLength(2); // start + done only
1081+
expect(events[0]).toMatchObject({ event: "start", deploymentId: liveDeployId });
1082+
expect(events[1]).toMatchObject({ event: "done", deployment: liveDeployId });
1083+
});
1084+
1085+
it("proceeds with deploy when manifest differs from live deployment", async () => {
1086+
const user = await createTestUser(env.db);
1087+
const project = await createTestProject(env.db, user.id, "dedupe-manifest-app");
1088+
mockCfApi();
1089+
1090+
// Insert a live deployment with a different manifest
1091+
const liveDeployId = "dpl_live_diff";
1092+
await env.db.insert(deployments).values({
1093+
id: liveDeployId,
1094+
project_id: project.id,
1095+
status: "live",
1096+
manifest: JSON.stringify({ version: 3, bindings: { d1: ["DB"] } }),
1097+
asset_hashes: JSON.stringify(assetManifest),
1098+
created_at: sql`now()`,
1099+
});
1100+
await env.db
1101+
.update(projects)
1102+
.set({ active_deployment_id: liveDeployId })
1103+
.where(eq(projects.id, project.id));
1104+
1105+
// Deploy with a different manifest (no d1 binding)
1106+
const form = buildDeployForm({ manifest, assets: { "index.html": "<h1>hi</h1>" } });
1107+
form.append("assetManifest", JSON.stringify(assetManifest));
1108+
form.append("preflight", "true");
1109+
1110+
const res = await app.fetch(
1111+
new Request(`http://localhost/projects/${project.id}/deploy`, {
1112+
method: "POST",
1113+
headers: { Authorization: `Bearer ${user.jwt}` },
1114+
body: form,
1115+
}),
1116+
env,
1117+
);
1118+
1119+
expect(res.status).toBe(200);
1120+
const events = parseDeployStream(await res.text());
1121+
expect(events.length).toBeGreaterThan(2);
1122+
expect(events.at(-1)).toMatchObject({ event: "done" });
9891123
});
9901124
});
9911125

0 commit comments

Comments
 (0)