Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
📝 WalkthroughWalkthroughAdds an Arweave fallback: service worker that proxies/falls back to ar-io.net, client-side registration component, URL-rewrite helpers, and config/CSP updates to allow Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant SW as Service Worker
participant Arweave as arweave.net
participant Fallback as ar-io.net
Browser->>SW: fetch(request to arweave.net)
activate SW
SW->>Arweave: fetch(original request)
alt original returns OK/opaque
Arweave-->>Browser: response
else original fails or non-OK
SW->>Fallback: fetch(rewritten request to ar-io.net)
alt fallback returns response
Fallback-->>Browser: response
else fallback fails
SW-->>Browser: return error response
end
end
deactivate SW
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (6)
public/arweave-fallback-sw.js (3)
4-11: Hostname comparison lacks case normalization.Unlike the TypeScript helper in
lib/arweave-fallback.ts(line 7), this function doesn't normalize the hostname to lowercase before comparison. While hostnames are typically lowercase, inconsistent handling could cause edge-case mismatches.♻️ Suggested fix
function isArweaveRequest(url) { try { const u = new URL(url); - return u.hostname === ARWEAVE_HOST || u.hostname === "www." + ARWEAVE_HOST; + const h = u.hostname.toLowerCase(); + return h === ARWEAVE_HOST || h === "www." + ARWEAVE_HOST; } catch { return false; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/arweave-fallback-sw.js` around lines 4 - 11, The hostname comparison in isArweaveRequest should normalize casing to avoid mismatches; update isArweaveRequest to call toLowerCase() on the parsed URL hostname and compare against ARWEAVE_HOST.toLowerCase() and ("www."+ARWEAVE_HOST).toLowerCase() (or precompute a lowercase www variant) so both sides are compared in lowercase while preserving the existing try/catch behavior.
13-22: Redundant host assignment.Setting
u.hostnamealready updates the hostname portion ofu.host. The subsequent assignment tou.hostimmediately overwrites it. While functionally correct, theu.hostnameassignment on line 16 is unnecessary.♻️ Simplified version
function toFallbackUrl(url) { try { const u = new URL(url); - u.hostname = FALLBACK_HOST; u.host = FALLBACK_HOST + (u.port ? ":" + u.port : ""); return u.toString(); } catch { return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/arweave-fallback-sw.js` around lines 13 - 22, In toFallbackUrl, the assignment to u.host is redundant because setting u.hostname already updates the host; remove the explicit u.host = ... line and just set u.hostname = FALLBACK_HOST (keep the try/catch and return logic intact) so the hostname is replaced without overwriting the host property redundantly.
35-45: Fallback triggers on any non-ok response, including 404s.The current logic falls back to
ar-io.netwhen the primary request returns a non-ok status (e.g., 404 Not Found). If the resource genuinely doesn't exist on Arweave, this causes an unnecessary retry to the fallback host, which will also likely 404.Consider preserving and returning non-ok responses directly instead of triggering fallback:
♻️ Suggested fix to only fallback on network errors
try { const res = await fetch(event.request, opts); - if (res.ok) return res; - } catch {} + return res; // Return any response (including 4xx/5xx) + } catch { + // Network error - try fallback + } const fallback = toFallbackUrl(event.request.url); if (!fallback) return fetch(event.request); try { return await fetch(fallback, opts); } catch { return fetch(event.request); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/arweave-fallback-sw.js` around lines 35 - 45, The current handler falls back to the ar-io URL whenever the primary fetch returns a non-ok response (e.g., 404); change the logic to only attempt the fallback when the initial fetch throws (network error) and to return non-ok responses directly. Specifically, in the block using fetch(event.request, opts) and checking res.ok, return res immediately when res is present (even if !res.ok) instead of proceeding to compute toFallbackUrl; only call toFallbackUrl and attempt fetch(fallback, opts) from the catch path that handles thrown errors (keep the existing catch that falls back to fetch(event.request) as a last resort). Ensure references: the initial fetch/res check and the toFallbackUrl fallback flow are updated accordingly.components/providers/ArweaveFallbackSwRegistration.tsx (1)
12-14: Silent error handling may hinder debugging.Registration failures are silently swallowed. While this is acceptable for production resilience, consider logging errors in development to aid debugging if the service worker fails to register.
♻️ Optional: Log errors in development
navigator.serviceWorker .register(SW_PATH, { scope: "/" }) - .catch(() => {}); + .catch((err) => { + if (process.env.NODE_ENV === "development") { + console.warn("Arweave fallback SW registration failed:", err); + } + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/ArweaveFallbackSwRegistration.tsx` around lines 12 - 14, The service worker registration call using navigator.serviceWorker.register(SW_PATH, { scope: "/" }) currently swallows errors in .catch(() => {}); modify the catch to capture the error (e) and log it in non-production environments so failures are visible during development—e.g., change the empty catch to a handler that checks NODE_ENV or a debug flag and calls console.error or your app logger with a message referencing SW_PATH and the caught error; keep silent behavior for production.lib/arweave-fallback.ts (2)
14-24: Same redundant host assignment pattern as service worker.Setting
u.hostnameon line 18 is immediately overwritten byu.hoston line 19.♻️ Simplified version
export function getArweaveFallbackUrl(url: string): string | null { if (!isArweaveUrl(url)) return null; try { const u = new URL(url); - u.hostname = FALLBACK_HOST; u.host = FALLBACK_HOST + (u.port ? ":" + u.port : ""); return u.toString(); } catch { return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/arweave-fallback.ts` around lines 14 - 24, The function getArweaveFallbackUrl currently sets u.hostname and then immediately overwrites it by setting u.host; remove the redundant assignment by keeping only the proper host construction—replace the two assignments with a single u.host = FALLBACK_HOST + (u.port ? ":" + u.port : ""); (or alternatively only set u.hostname and append the port separately), ensuring you reference getArweaveFallbackUrl and the FALLBACK_HOST constant when making the change.
1-2: Consider exporting constants for reuse and testing.The
ARWEAVE_HOSTandFALLBACK_HOSTconstants could be useful for consumers (e.g., tests, other modules that need to reference these hosts).♻️ Suggested change
-const ARWEAVE_HOST = "arweave.net"; -const FALLBACK_HOST = "ar-io.net"; +export const ARWEAVE_HOST = "arweave.net"; +export const FALLBACK_HOST = "ar-io.net";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/arweave-fallback.ts` around lines 1 - 2, Export the two host constants so other modules and tests can reuse them: make ARWEAVE_HOST and FALLBACK_HOST exported (e.g., add export before the const declarations or add a named export for those symbols) and ensure any internal usage within the same module still references the exported names (ARWEAVE_HOST, FALLBACK_HOST) so tests and consumers can import them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@components/providers/ArweaveFallbackSwRegistration.tsx`:
- Around line 12-14: The service worker registration call using
navigator.serviceWorker.register(SW_PATH, { scope: "/" }) currently swallows
errors in .catch(() => {}); modify the catch to capture the error (e) and log it
in non-production environments so failures are visible during development—e.g.,
change the empty catch to a handler that checks NODE_ENV or a debug flag and
calls console.error or your app logger with a message referencing SW_PATH and
the caught error; keep silent behavior for production.
In `@lib/arweave-fallback.ts`:
- Around line 14-24: The function getArweaveFallbackUrl currently sets
u.hostname and then immediately overwrites it by setting u.host; remove the
redundant assignment by keeping only the proper host construction—replace the
two assignments with a single u.host = FALLBACK_HOST + (u.port ? ":" + u.port :
""); (or alternatively only set u.hostname and append the port separately),
ensuring you reference getArweaveFallbackUrl and the FALLBACK_HOST constant when
making the change.
- Around line 1-2: Export the two host constants so other modules and tests can
reuse them: make ARWEAVE_HOST and FALLBACK_HOST exported (e.g., add export
before the const declarations or add a named export for those symbols) and
ensure any internal usage within the same module still references the exported
names (ARWEAVE_HOST, FALLBACK_HOST) so tests and consumers can import them.
In `@public/arweave-fallback-sw.js`:
- Around line 4-11: The hostname comparison in isArweaveRequest should normalize
casing to avoid mismatches; update isArweaveRequest to call toLowerCase() on the
parsed URL hostname and compare against ARWEAVE_HOST.toLowerCase() and
("www."+ARWEAVE_HOST).toLowerCase() (or precompute a lowercase www variant) so
both sides are compared in lowercase while preserving the existing try/catch
behavior.
- Around line 13-22: In toFallbackUrl, the assignment to u.host is redundant
because setting u.hostname already updates the host; remove the explicit u.host
= ... line and just set u.hostname = FALLBACK_HOST (keep the try/catch and
return logic intact) so the hostname is replaced without overwriting the host
property redundantly.
- Around line 35-45: The current handler falls back to the ar-io URL whenever
the primary fetch returns a non-ok response (e.g., 404); change the logic to
only attempt the fallback when the initial fetch throws (network error) and to
return non-ok responses directly. Specifically, in the block using
fetch(event.request, opts) and checking res.ok, return res immediately when res
is present (even if !res.ok) instead of proceeding to compute toFallbackUrl;
only call toFallbackUrl and attempt fetch(fallback, opts) from the catch path
that handles thrown errors (keep the existing catch that falls back to
fetch(event.request) as a last resort). Ensure references: the initial fetch/res
check and the toFallbackUrl fallback flow are updated accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/providers/ArweaveFallbackSwRegistration.tsxcomponents/providers/Providers.tsxconfig/nextConfig.tsconfig/securityHeaders.tslib/arweave-fallback.tspublic/arweave-fallback-sw.js
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/arweave-fallback-sw.js`:
- Line 5: The current check uses url.includes("arweave.net") which can match
arbitrary parts of the URL; update the conditional that uses the variable url
(in public/arweave-fallback-sw.js) to parse the request URL with new
URL(url).hostname and perform a strict match (e.g., hostname === "arweave.net"
or hostname.endsWith(".arweave.net") as appropriate) instead of includes,
keeping the existing event.request.mode === "navigate" early return behavior so
only true arweave hostnames are rewritten to the ar-io.net fallback.
- Around line 9-13: Restrict the fallback retry to safe HTTP methods by checking
event.request.method before attempting the fallback: allow only "GET" and
"HEAD". In the arweave-fallback-sw logic around opts, res, fallback and
FALLBACK_HOST, perform the initial fetch as-is but only compute and fetch the
fallback when event.request.method is "GET" or "HEAD" (otherwise return the
original response or null without retrying). This change prevents non-idempotent
methods from being retried against the fallback host.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
public/arweave-fallback-sw.js (1)
26-27: Regex replacement discards port, inconsistent withlib/arweave-fallback.ts.The regex
^(https?:\/\/)([^/]+)replaces the entire host (including port) with justFALLBACK_HOST. For URLs likehttps://arweave.net:8080/path, the fallback becomeshttps://ar-io.net/path, losing the port. Meanwhile,getArweaveFallbackUrlinlib/arweave-fallback.tspreserves the port.If port preservation isn't needed for this use case (standard HTTPS on port 443), this is fine. Otherwise, consider using URL parsing:
♻️ Suggested fix to preserve port
- const fallback = event.request.url.replace(/^(https?:\/\/)([^/]+)/, `$1${FALLBACK_HOST}`); + const u = new URL(event.request.url); + u.hostname = FALLBACK_HOST; + const fallback = u.toString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@public/arweave-fallback-sw.js` around lines 26 - 27, The fallback URL construction uses a regex replacement in the event handler (the const fallback = event.request.url.replace(...) that creates fallback and is returned via Response.redirect) which drops any port from the original URL; update this to parse the original URL (e.g., new URL(event.request.url)) and reconstruct the fallback host preserving the original port when present (or otherwise mirror the behavior of getArweaveFallbackUrl in lib/arweave-fallback.ts) so the resulting fallback keeps the port if it existed; ensure you update the code that sets fallback and leave the Response.redirect(fallback, 302) call unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@public/arweave-fallback-sw.js`:
- Around line 26-27: The fallback URL construction uses a regex replacement in
the event handler (the const fallback = event.request.url.replace(...) that
creates fallback and is returned via Response.redirect) which drops any port
from the original URL; update this to parse the original URL (e.g., new
URL(event.request.url)) and reconstruct the fallback host preserving the
original port when present (or otherwise mirror the behavior of
getArweaveFallbackUrl in lib/arweave-fallback.ts) so the resulting fallback
keeps the port if it existed; ensure you update the code that sets fallback and
leave the Response.redirect(fallback, 302) call unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
components/providers/ArweaveFallbackSwRegistration.tsxlib/arweave-fallback.tspublic/arweave-fallback-sw.js
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@public/arweave-fallback-sw.js`:
- Around line 28-37: The fallback currently calls fetch(fallbackUrl) which loses
the original Request's mode/credentials/headers; change fetchFallback to create
a new Request for the fallback URL that preserves the original request semantics
(e.g. use new Request(fallbackUrl, request) or explicitly copy request.method,
request.headers, request.mode, request.credentials, cache, redirect,
referrerPolicy, integrity, keepalive and body/clone when needed) and pass that
Request to fetch so mode/no-cors and credentials are preserved; update the
function fetchFallback to build and fetch this new Request.
Comment on lines
+28
to
+37
| function fetchFallback(request) { | ||
| const originalUrl = new URL(request.url); | ||
| const fallbackUrl = | ||
| "https://ar-io.net" + | ||
| originalUrl.pathname + | ||
| originalUrl.search + | ||
| originalUrl.hash; | ||
|
|
||
| return fetch(fallbackUrl); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Fetch API default request properties mode credentials when calling fetch(url) vs original request semantics
💡 Result:
When you call fetch(url) with a string URL, the Fetch spec creates a new Request using default “request semantics” for any RequestInit fields you didn’t provide:
method:GET(and no request body)mode:"cors"(so cross-origin requests use CORS; same-origin also works) [1][2]credentials:"same-origin"(send cookies/auth headers only for same-origin requests; not for cross-origin) [1][3]- (Common other defaults:
redirect: "follow",cache: "default".) [1]
In contrast, when you call fetch(request) with an existing Request object, it uses that object’s already-set properties (including mode, credentials, etc.), unless you explicitly override them in the second argument (which “clones” with overrides per the Fetch algorithms). The key difference is: string URL ⇒ defaults apply; Request object ⇒ its stored semantics apply. [1][3]
Sources:
[1] WHATWG Fetch Standard (request creation defaults for mode/credentials) (fetch.spec.whatwg.org)
[2] MDN: Using the Fetch API (default mode is cors) (developer.mozilla.org)
[3] MDN: Request.credentials (default is same-origin) (developer.mozilla.org)
Citations:
- 1: https://fetch.spec.whatwg.org/?from=20423&from_column=20423&utm_source=openai
- 2: https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch?utm_source=openai
- 3: https://developer.mozilla.org/docs/Web/API/Request/credentials?utm_source=openai
🏁 Script executed:
cd public && cat -n arweave-fallback-sw.js | head -50Repository: 6529-Collections/6529seize-frontend
Length of output: 1232
Preserve request mode/semantics when calling the fallback host.
On Line 36, fetch(fallbackUrl) creates a new request with default fetch options (specifically mode: "cors" and credentials: "same-origin"). This can break cross-origin asset requests that were originally made with no-cors mode, since the fallback request will now attempt standard CORS negotiation instead of requesting an opaque response. If ar-io.net lacks the appropriate CORS headers, the fallback will fail.
🔧 Proposed fix
function fetchFallback(request) {
const originalUrl = new URL(request.url);
const fallbackUrl =
"https://ar-io.net" +
originalUrl.pathname +
originalUrl.search +
originalUrl.hash;
- return fetch(fallbackUrl);
+ return fetch(fallbackUrl, {
+ method: request.method,
+ mode: request.mode,
+ cache: request.cache,
+ redirect: request.redirect,
+ referrer: request.referrer,
+ referrerPolicy: request.referrerPolicy,
+ integrity: request.integrity,
+ credentials: "omit",
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@public/arweave-fallback-sw.js` around lines 28 - 37, The fallback currently
calls fetch(fallbackUrl) which loses the original Request's
mode/credentials/headers; change fetchFallback to create a new Request for the
fallback URL that preserves the original request semantics (e.g. use new
Request(fallbackUrl, request) or explicitly copy request.method,
request.headers, request.mode, request.credentials, cache, redirect,
referrerPolicy, integrity, keepalive and body/clone when needed) and pass that
Request to fetch so mode/no-cors and credentials are preserved; update the
function fetchFallback to build and fetch this new Request.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores