Skip to content

Commit 19aead0

Browse files
committed
fix(images): prevent URL expiration by immediate download
Implemented two-phase fix for Issue #94: **Phase 1: Reorder image processing (CRITICAL FIX)** - Moved image download to immediately after markdown conversion - Images now process before emoji and callout processing - Reduces time gap from 3-7 seconds to <1 second per page - In generateBlocks.ts:processSinglePage(), moved processAndReplaceImages() from line 320 to line 287 (right after toMarkdownString()) **Phase 2: Expired URL detection and logging** - Added isExpiredUrlError() helper to detect 403 expired signatures - Enhanced error logging to identify expired URLs specifically - Detects AWS S3 "SignatureDoesNotMatch" and other expiration errors - Provides clear diagnostic message when URLs expire **Testing** - Added comprehensive test suite: imageUrlExpiration.test.ts - Added expired URL detection tests: expiredUrlDetection.test.ts - All existing tests pass (downloadImage.test.ts: 9/9) - Expired URL detection tests pass (17/17) **Impact** - Notion image URLs expire after 1 hour - Previous flow: fetch → emoji → callouts → images (3-7s delay) - New flow: fetch → images → emoji → callouts (<1s delay) - With 50 pages at 5 concurrent, saves ~30-70 seconds of cumulative delay - Prevents URLs from expiring during batch processing Fixes #94 See IMAGE_URL_EXPIRATION_SPEC.md for full technical specification
1 parent ca54ff3 commit 19aead0

File tree

4 files changed

+862
-8
lines changed

4 files changed

+862
-8
lines changed
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
/**
2+
* Tests for Expired URL Detection (Phase 2)
3+
*
4+
* Tests the isExpiredUrlError() helper function that detects
5+
* when a 403 error is specifically due to an expired Notion image URL.
6+
*/
7+
8+
import { describe, it, expect } from "vitest";
9+
import { isExpiredUrlError } from "../imageProcessing";
10+
11+
describe("Expired URL Detection", () => {
12+
describe("isExpiredUrlError()", () => {
13+
it("should return true for 403 with SignatureDoesNotMatch", () => {
14+
const error = {
15+
response: {
16+
status: 403,
17+
data: "SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided",
18+
},
19+
};
20+
21+
expect(isExpiredUrlError(error)).toBe(true);
22+
});
23+
24+
it("should return true for 403 with Request has expired", () => {
25+
const error = {
26+
response: {
27+
status: 403,
28+
data: "Request has expired",
29+
},
30+
};
31+
32+
expect(isExpiredUrlError(error)).toBe(true);
33+
});
34+
35+
it("should return true for 403 with expired in message", () => {
36+
const error = {
37+
response: {
38+
status: 403,
39+
data: "The URL has expired",
40+
},
41+
};
42+
43+
expect(isExpiredUrlError(error)).toBe(true);
44+
});
45+
46+
it("should return true for 403 with Signature expired", () => {
47+
const error = {
48+
response: {
49+
status: 403,
50+
data: "Signature expired: 20251127T120000Z is now earlier than 20251127T130000Z",
51+
},
52+
};
53+
54+
expect(isExpiredUrlError(error)).toBe(true);
55+
});
56+
57+
it("should return true for expired in error message", () => {
58+
const error = {
59+
message: "Request failed: URL expired",
60+
response: {
61+
status: 403,
62+
data: "",
63+
},
64+
};
65+
66+
expect(isExpiredUrlError(error)).toBe(true);
67+
});
68+
69+
it("should return true for signature in error message", () => {
70+
const error = {
71+
message: "signature validation failed",
72+
response: {
73+
status: 403,
74+
data: "",
75+
},
76+
};
77+
78+
expect(isExpiredUrlError(error)).toBe(true);
79+
});
80+
81+
it("should return false for 403 without expiration indicators", () => {
82+
const error = {
83+
response: {
84+
status: 403,
85+
data: "Access Denied",
86+
},
87+
};
88+
89+
expect(isExpiredUrlError(error)).toBe(false);
90+
});
91+
92+
it("should return false for 404 error", () => {
93+
const error = {
94+
response: {
95+
status: 404,
96+
data: "Not Found",
97+
},
98+
};
99+
100+
expect(isExpiredUrlError(error)).toBe(false);
101+
});
102+
103+
it("should return false for 500 error", () => {
104+
const error = {
105+
response: {
106+
status: 500,
107+
data: "Internal Server Error",
108+
},
109+
};
110+
111+
expect(isExpiredUrlError(error)).toBe(false);
112+
});
113+
114+
it("should return false for network errors without status", () => {
115+
const error = {
116+
message: "Network Error",
117+
code: "ECONNREFUSED",
118+
};
119+
120+
expect(isExpiredUrlError(error)).toBe(false);
121+
});
122+
123+
it("should handle error with no response", () => {
124+
const error = {
125+
message: "Something went wrong",
126+
};
127+
128+
expect(isExpiredUrlError(error)).toBe(false);
129+
});
130+
131+
it("should handle null/undefined error", () => {
132+
expect(isExpiredUrlError(null)).toBe(false);
133+
expect(isExpiredUrlError(undefined)).toBe(false);
134+
});
135+
136+
it("should handle error with object response data", () => {
137+
const error = {
138+
response: {
139+
status: 403,
140+
data: {
141+
error: "SignatureDoesNotMatch",
142+
message: "The signature does not match",
143+
},
144+
},
145+
};
146+
147+
expect(isExpiredUrlError(error)).toBe(true);
148+
});
149+
150+
it("should be case-insensitive for expiration indicators", () => {
151+
const error1 = {
152+
response: {
153+
status: 403,
154+
data: "SIGNATUREDOESNOTMATCH",
155+
},
156+
};
157+
158+
const error2 = {
159+
response: {
160+
status: 403,
161+
data: "request has EXPIRED",
162+
},
163+
};
164+
165+
expect(isExpiredUrlError(error1)).toBe(true);
166+
expect(isExpiredUrlError(error2)).toBe(true);
167+
});
168+
});
169+
170+
describe("Real-world AWS S3 Error Formats", () => {
171+
it("should detect AWS S3 SignatureDoesNotMatch XML response", () => {
172+
const error = {
173+
response: {
174+
status: 403,
175+
data: `<?xml version="1.0" encoding="UTF-8"?>
176+
<Error>
177+
<Code>SignatureDoesNotMatch</Code>
178+
<Message>The request signature we calculated does not match the signature you provided.</Message>
179+
<RequestId>ABC123</RequestId>
180+
</Error>`,
181+
},
182+
};
183+
184+
expect(isExpiredUrlError(error)).toBe(true);
185+
});
186+
187+
it("should detect AWS S3 RequestTimeTooSkewed error", () => {
188+
const error = {
189+
response: {
190+
status: 403,
191+
data: `<?xml version="1.0" encoding="UTF-8"?>
192+
<Error>
193+
<Code>RequestTimeTooSkewed</Code>
194+
<Message>The difference between the request time and the server's time is too large.</Message>
195+
</Error>`,
196+
},
197+
};
198+
199+
// This should be false as it's not an expiration issue
200+
expect(isExpiredUrlError(error)).toBe(false);
201+
});
202+
203+
it("should detect AWS S3 AccessDenied without expiration", () => {
204+
const error = {
205+
response: {
206+
status: 403,
207+
data: `<?xml version="1.0" encoding="UTF-8"?>
208+
<Error>
209+
<Code>AccessDenied</Code>
210+
<Message>Access Denied</Message>
211+
</Error>`,
212+
},
213+
};
214+
215+
expect(isExpiredUrlError(error)).toBe(false);
216+
});
217+
});
218+
});

0 commit comments

Comments
 (0)