feat: add serverlist - #243
Conversation
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…preset to serverlist
…pagination to a standalone component
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
birdflop-com | bddc927 | Commit Preview URL Branch Preview URL |
Aug 01 2026, 06:05 PM |
📝 WalkthroughWalkthroughAdds a complete Minecraft server list with persistent schema, server submission and editing, filtering, live status checks, voting, reporting, verification, BBCode rendering, and plugin synchronization with linked servers. ChangesServer List Platform
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant ServerList
participant ServerStatus
participant Database
Visitor->>ServerList: request filtered server list
ServerList->>Database: query servers and vote totals
ServerList->>ServerStatus: fetch live statuses
ServerStatus-->>ServerList: return normalized statuses
ServerList-->>Visitor: render ranked server cards
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/resources/plugins/index.tsx (1)
177-233: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftNo debounce on writes triggered for every store mutation.
deepTrack(track, pluginsStore)reruns this task on any change topluginsStore, including UI-only changes like switchingopenServerorfilter. For authenticated users, each rerun now also callssetUserDataand, when a server is linked,updateServerPlugins. Previously this task only wrote tolocalStorage, which is cheap; it now issues network requests to persist the whole store on every interaction, with no debouncing and no guard against overlapping in-flight writes if the store changes again before a previous write resolves.Debounce the network persistence, or split UI-only state (
openServer,filter) from plugin-content state so trivial interactions do not trigger asetUserData/updateServerPluginsround trip.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/resources/plugins/index.tsx` around lines 177 - 233, Debounce the authenticated network persistence in the pluginsStore task so UI-only mutations such as openServer or filter do not trigger immediate requests. Keep localStorage persistence responsive, but schedule setUserData and updateServerPlugins through a shared debounce that coalesces changes and prevents overlapping in-flight writes, using the existing pluginsStore task and persistence symbols.
🧹 Nitpick comments (2)
src/components/ServerList/ServerCard.tsx (1)
53-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the address formatting into a helper.
The nested ternary computes the Java address and the Bedrock address in one expression.
src/routes/serverlist/[slug]/index.tsxlines 116-121 repeats the same rule withDEFAULT_JAVA_PORTandDEFAULT_BEDROCK_PORT. Move the logic to a shared function insrc/util/serverlist/and use the constants here too, so the two views cannot diverge.♻️ Suggested shape
// src/util/serverlist/address.ts export function formatAddress( host: string, port: number | null, defaultPort: number ): string { return port && port !== defaultPort ? `${host}:${port}` : host; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/ServerList/ServerCard.tsx` around lines 53 - 65, Extract the nested address-formatting logic from the ServerCard displayIp calculation into a shared formatAddress helper under src/util/serverlist/, and reuse it for both Java and Bedrock addresses. Update ServerCard and the server-list route to use DEFAULT_JAVA_PORT and DEFAULT_BEDROCK_PORT through this helper, preserving omission of default ports and formatting only the selected primary host.src/routes/serverlist/[slug]/index.tsx (1)
90-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
Outputfor the copy control, and handle the clipboard rejection.
navigator.clipboard?.writeText(address)returns an unhandled promise. A denied clipboard permission fails silently, and the user gets no feedback.src/components/Elements/Output.tsxalready implements copy with notifications. Use that component here instead of the local button.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/routes/serverlist/`[slug]/index.tsx around lines 90 - 111, Replace the local copy button in ConnectRow with the existing Output component, passing address as its displayed/copyable value and preserving the current row styling and label. Remove the direct navigator.clipboard call so clipboard failures are handled by Output’s notification behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@drizzle/schema.ts`:
- Around line 159-223: Prevent votifierToken from reaching clients by replacing
whole-row selections of servers in the server-list query flow and the server
detail route with explicit public-column projections. Exclude votifierToken from
those projections while preserving all fields required by the listing UI; retain
access only in privileged backend paths that need to deliver votes.
In `@src/components/Elements/Output.tsx`:
- Around line 38-52: Update the compact branch in the Output component to use a
keyboard-accessible button with button semantics while preserving the existing
click behavior, styling, value, and copy icon. Keep the control labeled with the
translated “Copy IP” title by routing that string through inlineTranslate,
consistent with the other localized strings in the component.
- Around line 23-36: Update the onClick handler in Output.tsx so
notifications.push(notification.toJSON()) runs after
navigator.clipboard.writeText(value) resolves or rejects, ensuring the
catch-updated failure notification snapshot is pushed. Preserve the green
success notification for successful copies and the existing error details and
red styling for failures.
In `@src/components/ServerList/StatusBadge.tsx`:
- Around line 13-39: Update the players display condition in the StatusBadge
component so it narrows status before accessing player data, and ensure the
status.players.online and status.players.max reads remain safe when status or
players is null. Preserve the existing rendering behavior for online servers
with showPlayers enabled.
In `@src/components/ServerList/Turnstile.tsx`:
- Around line 57-76: Update the Turnstile effect around render and cleanup so
the script element and load-handler state are accessible to ctx.cleanup. Remove
the load listener during cleanup and guard render with an active-mounted flag,
ensuring the script cannot create a widget after unmount while preserving normal
immediate and script-load rendering.
In `@src/routes/resources/plugins/index.tsx`:
- Around line 134-152: Update the database plugin-state condition in the
useVisibleTask$ initialization flow to distinguish missing data from an
intentionally empty sync. Treat any defined dbPlugins value, including one with
an empty servers object, as authoritative and restore its stored fields; only
enter the localStorage migration/setUserData fallback when dbPlugins itself is
undefined.
In `@src/routes/serverlist/`[slug]/index.tsx:
- Around line 72-76: Annotate the session value retrieved in the
server-management checks as Session | null, using the existing Session type from
src/util/dataUtils.ts. Apply this in src/routes/serverlist/[slug]/index.tsx
lines 72-76 and src/routes/serverlist/[slug]/edit/index.tsx lines 21-24 before
accessing session.user, without changing the canManage logic.
In `@src/routes/serverlist/index.tsx`:
- Around line 76-86: Update the error handling in the server-loading function to
log the caught error server-side, but replace error: String(err) with a fixed
user-safe message before the result reaches the page rendering at line 356. Keep
the existing empty rows, total, statuses, liveRefined, and params behavior
unchanged.
- Around line 31-35: Validate the numeric results used to derive page and
perPage in the server-list pagination setup. Update the page and perPage parsing
expressions so non-numeric query values fall back to their existing defaults
before applying the Math.max/Math.min bounds, ensuring valid values reach limit,
offset, and totalPages.
In `@src/util/serverlist/actions.ts`:
- Around line 271-301: Update reportServer to validate a Turnstile token using
the existing verifyTurnstile pattern from voteForServer, and enforce the same
24-hour username/IP cooldown before inserting into serverReports. Preserve the
existing validation and response behavior, and reject submissions that fail
CAPTCHA or rate-limit checks before the database insert.
- Around line 352-370: Update setServerVerified to look up the server by
serverId before performing the update, following the existing
updateServerPlugins pattern. Return the established not-found failure response
when no matching server exists, and only return success after updating an
existing row.
- Around line 47-370: Add a shared session accessor that casts
sharedMap.get('session') to Session | null, then replace the direct session
lookups in createServer, updateServer, deleteServer, reportServer, and
updateServerPlugins with this helper. Import Session from `@auth/qwik` and
preserve the existing null-safe authorization behavior.
- Line 88: Standardize all isAdmin invocations in the caller actions across the
serverlist and data-utils modules. Use the bound server$ call form consistently
so each admin check receives the request context and can access this.sharedMap,
including the invocation near the existing const admin assignment.
In `@src/util/serverlist/bbcode.ts`:
- Around line 73-105: Type every regex replacement callback parameter currently
inferred as any in the BBCode conversion chain, including color/size, URL
href/label, image src, and list callback parameters, as string. Update the
callbacks around the existing safeColor, safeSize, safeUrl, and related list
handling without changing their behavior.
In `@src/util/serverlist/queries.ts`:
- Around line 96-120: Prevent sensitive server and owner fields from reaching
public Qwik loader payloads: in src/util/serverlist/queries.ts lines 96-120 and
src/routes/serverlist/[slug]/index.tsx lines 36-88, replace full-row selections
with explicit public column projections excluding servers.votifierToken and
owner PII; in src/routes/serverlist/[slug]/edit/index.tsx lines 12-27, only
return listing data when canManage is true and exclude votifierToken; in
drizzle/schema.ts lines 159-223, either move Votifier credentials into a
separate table or document the invariant that votifierToken is never selected by
public read paths.
In `@src/util/serverlist/status.ts`:
- Around line 57-67: Update the fetch call in getServerStatus to pass an
AbortSignal.timeout using the appropriate request duration, and ensure an
aborted request follows the existing fallback path. Preserve the current
response validation and McStatusResponse handling for requests that complete
normally.
In `@src/util/serverlist/turnstile.ts`:
- Around line 27-44: Update the Turnstile verification fetch in verifyTurnstile
to use an AbortController with a 5-second timeout, passing its signal to fetch
and aborting when the timer expires. Clear the timeout after the fetch completes
or fails, while preserving the existing success, failure, and catch handling.
In `@src/util/serverlist/validation.ts`:
- Around line 75-82: Update parsePort to validate string inputs after trimming,
requiring the complete value to consist only of decimal digits before
conversion. Replace the partial parseInt behavior with Number() after this
check, while preserving existing undefined, empty, null, integer, and 1–65535
range handling.
- Around line 94-95: Update server destination validation around HOST_RE to
resolve hostnames and reject private or reserved IP ranges, including localhost,
loopback, RFC 1918, link-local, unspecified, and equivalent IPv6 addresses,
before any status check or Votifier message is sent. Ensure status.ts validates
the resolved destination before fetch() and sendVotifierV2() performs the same
protection before creating the cloudflare:sockets TCP connection.
In `@src/util/serverlist/votifier.ts`:
- Around line 75-91: Update the socket declaration and the dynamically imported
connect return type in the votifier flow to use ReadableStream<Uint8Array> and
WritableStream<Uint8Array> instead of unparameterized stream types, ensuring
reader.read().value is typed for TextDecoder.decode without call-site casts.
---
Outside diff comments:
In `@src/routes/resources/plugins/index.tsx`:
- Around line 177-233: Debounce the authenticated network persistence in the
pluginsStore task so UI-only mutations such as openServer or filter do not
trigger immediate requests. Keep localStorage persistence responsive, but
schedule setUserData and updateServerPlugins through a shared debounce that
coalesces changes and prevents overlapping in-flight writes, using the existing
pluginsStore task and persistence symbols.
---
Nitpick comments:
In `@src/components/ServerList/ServerCard.tsx`:
- Around line 53-65: Extract the nested address-formatting logic from the
ServerCard displayIp calculation into a shared formatAddress helper under
src/util/serverlist/, and reuse it for both Java and Bedrock addresses. Update
ServerCard and the server-list route to use DEFAULT_JAVA_PORT and
DEFAULT_BEDROCK_PORT through this helper, preserving omission of default ports
and formatting only the selected primary host.
In `@src/routes/serverlist/`[slug]/index.tsx:
- Around line 90-111: Replace the local copy button in ConnectRow with the
existing Output component, passing address as its displayed/copyable value and
preserving the current row styling and label. Remove the direct
navigator.clipboard call so clipboard failures are handled by Output’s
notification behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b7489f4-3bea-4f91-8879-3d5c06bafeb4
📒 Files selected for processing (43)
drizzle/migrations/0006_reflective_tarantula.sqldrizzle/migrations/0007_burly_madame_web.sqldrizzle/migrations/0008_safe_nightshade.sqldrizzle/migrations/0009_soft_krista_starr.sqldrizzle/migrations/0010_stale_sumo.sqldrizzle/migrations/0011_wet_stranger.sqldrizzle/migrations/meta/0006_snapshot.jsondrizzle/migrations/meta/0007_snapshot.jsondrizzle/migrations/meta/0008_snapshot.jsondrizzle/migrations/meta/0009_snapshot.jsondrizzle/migrations/meta/0010_snapshot.jsondrizzle/migrations/meta/0011_snapshot.jsondrizzle/migrations/meta/_journal.jsondrizzle/schema.tssrc/components/Elements/Nav.tsxsrc/components/Elements/Output.tsxsrc/components/ServerList/BBCodeEditor.tsxsrc/components/ServerList/ServerCard.tsxsrc/components/ServerList/ServerControls.tsxsrc/components/ServerList/ServerForm.tsxsrc/components/ServerList/StatusBadge.tsxsrc/components/ServerList/Turnstile.tsxsrc/components/ServerList/VoteSection.tsxsrc/components/rgbirdflop/presets/MyPrivatePresets.tsxsrc/routes/plugin@auth.tssrc/routes/resources/plugins/index.tsxsrc/routes/resources/rgb/index.tsxsrc/routes/serverlist/[slug]/edit/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/routes/serverlist/index.tsxsrc/routes/serverlist/submit/index.tsxsrc/types.d.tssrc/util/dataUtils.tssrc/util/plugins/types.tssrc/util/serverlist/__tests__/validation.test.tssrc/util/serverlist/actions.tssrc/util/serverlist/bbcode.tssrc/util/serverlist/constants.tssrc/util/serverlist/queries.tssrc/util/serverlist/status.tssrc/util/serverlist/turnstile.tssrc/util/serverlist/validation.tssrc/util/serverlist/votifier.ts
| const onClick = () => { | ||
| const notification = new Notification() | ||
| .setTitle(copiedTitle) | ||
| .setDescription(copiedDescription) | ||
| .setBgColor('lum-grad-bg-green/50'); | ||
| navigator.clipboard.writeText(value).catch((err: unknown) => { | ||
| notification | ||
| .setTitle(copyFailedTitle) | ||
| .setDescription(err instanceof Error ? err.message : String(err)) | ||
| .setBgColor('lum-grad-bg-red/50') | ||
| .setPersist(true); | ||
| }); | ||
| notifications.push(notification.toJSON()); | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
The copy-failure notification never reaches the user.
notifications.push(notification.toJSON()) runs synchronously and pushes a plain snapshot object. The catch callback runs later and mutates the notification instance, not the pushed snapshot. On a clipboard failure the user still sees the green "copied" notification. Push the notification inside the promise resolution instead.
🐛 Proposed fix
const onClick = () => {
const notification = new Notification()
.setTitle(copiedTitle)
.setDescription(copiedDescription)
.setBgColor('lum-grad-bg-green/50');
- navigator.clipboard.writeText(value).catch((err: unknown) => {
- notification
- .setTitle(copyFailedTitle)
- .setDescription(err instanceof Error ? err.message : String(err))
- .setBgColor('lum-grad-bg-red/50')
- .setPersist(true);
- });
- notifications.push(notification.toJSON());
+ navigator.clipboard
+ .writeText(value)
+ .then(() => notifications.push(notification.toJSON()))
+ .catch((err: unknown) => {
+ notifications.push(
+ notification
+ .setTitle(copyFailedTitle)
+ .setDescription(err instanceof Error ? err.message : String(err))
+ .setBgColor('lum-grad-bg-red/50')
+ .setPersist(true)
+ .toJSON()
+ );
+ });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const onClick = () => { | |
| const notification = new Notification() | |
| .setTitle(copiedTitle) | |
| .setDescription(copiedDescription) | |
| .setBgColor('lum-grad-bg-green/50'); | |
| navigator.clipboard.writeText(value).catch((err: unknown) => { | |
| notification | |
| .setTitle(copyFailedTitle) | |
| .setDescription(err instanceof Error ? err.message : String(err)) | |
| .setBgColor('lum-grad-bg-red/50') | |
| .setPersist(true); | |
| }); | |
| notifications.push(notification.toJSON()); | |
| }; | |
| const onClick = () => { | |
| const notification = new Notification() | |
| .setTitle(copiedTitle) | |
| .setDescription(copiedDescription) | |
| .setBgColor('lum-grad-bg-green/50'); | |
| navigator.clipboard | |
| .writeText(value) | |
| .then(() => notifications.push(notification.toJSON())) | |
| .catch((err: unknown) => { | |
| notifications.push( | |
| notification | |
| .setTitle(copyFailedTitle) | |
| .setDescription(err instanceof Error ? err.message : String(err)) | |
| .setBgColor('lum-grad-bg-red/50') | |
| .setPersist(true) | |
| .toJSON() | |
| ); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/Elements/Output.tsx` around lines 23 - 36, Update the onClick
handler in Output.tsx so notifications.push(notification.toJSON()) runs after
navigator.clipboard.writeText(value) resolves or rejects, ensuring the
catch-updated failure notification snapshot is pushed. Preserve the green
success notification for successful copies and the existing error details and
red styling for failures.
| const online = !!status?.online; | ||
| return ( | ||
| <div class="flex items-center gap-2 text-sm"> | ||
| <span | ||
| class={{ | ||
| 'flex items-center gap-1.5 font-semibold': true, | ||
| 'text-green-400': online, | ||
| 'text-lum-text-secondary': !online, | ||
| }} | ||
| > | ||
| <Circle | ||
| size={10} | ||
| class={ | ||
| online | ||
| ? 'fill-green-400 text-green-400' | ||
| : 'fill-lum-text-secondary text-lum-text-secondary' | ||
| } | ||
| /> | ||
| {online ? 'Online' : 'Offline'} | ||
| </span> | ||
| {online && showPlayers && ( | ||
| <span class="text-lum-text-secondary flex items-center gap-1.5"> | ||
| <Users size={14} /> | ||
| {status.players.online.toLocaleString()} | ||
| <span class="opacity-50"> | ||
| / {status.players.max.toLocaleString()} | ||
| </span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -H -t f 'tsconfig*.json' --exec sh -c 'echo "== $1"; cat "$1"' _ {}Repository: birdflop/web
Length of output: 2068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -H 'StatusBadge.tsx' .
echo "== file outline =="
ast-grep outline src/components/ServerList/StatusBadge.tsx --view expanded || true
echo "== relevant file contents =="
cat -n src/components/ServerList/StatusBadge.tsx
echo "== search ServerStatus definition =="
rg -n "ServerStatus|type .*Status|interface .*Status" src packages -g '*.ts' -g '*.tsx' || trueRepository: birdflop/web
Length of output: 3579
🌐 Web query:
TypeScript strictNullChecks boolean variable narrow null inside conditional non-null assertion
💡 Result:
When strictNullChecks is enabled, TypeScript uses control flow-based type analysis to narrow variables [1]. When you perform a conditional check on a variable that might be null or undefined (e.g., if (variable)), TypeScript narrows the type within that block based on truthiness [2][3][4]. To safely handle a variable that could be null or undefined, you should prefer explicit type guards over non-null assertions whenever possible [5][6]. 1. Truthiness Narrowing: Using if (variable) will narrow the type by removing null and undefined, as they are falsy values [2][7][4]. However, be cautious if your variable could be an empty string or the number 0, as these are also falsy and would be removed by this check [2]. 2. Explicit Null Checks: Using if (variable!== null && variable!== undefined) or the more concise if (variable!= null) is the recommended way to explicitly guard against null and undefined [5][4]. This is safe, clear, and specifically targets the nullability issue without inadvertently excluding other valid values [5]. 3. Non-null Assertion Operator: The non-null assertion operator (!) is a postfix operator that tells the compiler to treat a value as non-null/non-undefined [5][6]. It does not perform a runtime check and is essentially an assertion by the developer [5][6]. Because it bypasses type checking, it should only be used when you are absolutely certain the value is present and the compiler cannot infer it [5][6]. Using it inside a conditional to "fix" a type error is generally discouraged because it creates a risk of runtime errors if your assumption is wrong [6]. In summary, instead of using a non-null assertion inside a conditional, you should use an explicit null check or a type guard to allow TypeScript to correctly narrow the type for you [5][4]. This leverages the language's safety features rather than overriding them [5].
Citations:
- 1: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0%2ehtml
- 2: https://www.typescriptlang.org/docs/handbook/2/narrowing.html
- 3: https://github.com/microsoft/TypeScript/wiki/Reference-Checker-Widening-Narrowing
- 4: https://basarat.gitbook.io/typescript/type-system/typeguard
- 5: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-2-0.html
- 6: https://basarat.gitbook.io/typescript/intro/strictnullchecks
- 7: https://www.typescriptlang.org/docs/handbook/2/narrowing
Narrow status before reading its players.
online is a boolean, so {online && showPlayers && ...} does not narrow status from ServerStatus | null; status.players can still report as possibly null. Use optional chaining in the conditional and on the reads, or narrow status before the block.
🔧 Proposed fix
- {online && showPlayers && (
+ {status?.online && showPlayers && (
<span class="text-lum-text-secondary flex items-center gap-1.5">
<Users size={14} />
{status.players.online.toLocaleString()}
<span class="opacity-50">
- / {status.players.max.toLocaleString()}
+ / {status.max.toLocaleString()}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ServerList/StatusBadge.tsx` around lines 13 - 39, Update the
players display condition in the StatusBadge component so it narrows status
before accessing player data, and ensure the status.players.online and
status.players.max reads remain safe when status or players is null. Preserve
the existing rendering behavior for online servers with showPlayers enabled.
| if (window.turnstile) { | ||
| render(); | ||
| } else { | ||
| // Load the script once; reuse it across widgets. | ||
| let script = document.querySelector<HTMLScriptElement>( | ||
| `script[src="${SCRIPT_SRC}"]` | ||
| ); | ||
| if (!script) { | ||
| script = document.createElement('script'); | ||
| script.src = SCRIPT_SRC; | ||
| script.async = true; | ||
| document.head.appendChild(script); | ||
| } | ||
| script.addEventListener('load', render); | ||
| } | ||
|
|
||
| ctx.cleanup(() => { | ||
| if (widgetId && window.turnstile) window.turnstile.remove(widgetId); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove the load listener on cleanup, and skip rendering after unmount.
If the component unmounts before the Turnstile script loads, the load listener still fires and calls render(). The widget is then created after ctx.cleanup ran, so window.turnstile.remove is never called for it. The listener also keeps render and the container reference alive.
🔧 Proposed fix
let widgetId: string | undefined;
+ let disposed = false;
const render = () => {
- if (!window.turnstile || !containerRef.value) return;
+ if (disposed || !window.turnstile || !containerRef.value) return;
widgetId = window.turnstile.render(containerRef.value, {
@@
- script.addEventListener('load', render);
+ script.addEventListener('load', render, { once: true });
}
ctx.cleanup(() => {
+ disposed = true;
+ script?.removeEventListener('load', render);
if (widgetId && window.turnstile) window.turnstile.remove(widgetId);
});Declare script outside the else branch so cleanup can reference it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/ServerList/Turnstile.tsx` around lines 57 - 76, Update the
Turnstile effect around render and cleanup so the script element and
load-handler state are accessible to ctx.cleanup. Remove the load listener
during cleanup and guard render with an active-mounted flag, ensuring the script
cannot create a widget after unmount while preserving normal immediate and
script-load rendering.
| try { | ||
| const res = await fetch(url, { | ||
| headers: { 'User-Agent': 'birdflop.com server list' }, | ||
| // Cloudflare-specific edge caching so repeated views are cheap and we | ||
| // stay friendly to mcstatus.io rate limits. | ||
| cf: { cacheTtl: CACHE_TTL_SECONDS, cacheEverything: true }, | ||
| }); | ||
|
|
||
| if (!res.ok) return fallback; | ||
|
|
||
| const data: McStatusResponse = await res.json(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add a timeout to the mcstatus.io request.
fetch has no abort signal. src/util/serverlist/queries.ts calls getServerStatus for up to CANDIDATE_CAP (75) servers per page view. If mcstatus.io is slow, every list request blocks until the platform limit, and page rendering stalls. Add AbortSignal.timeout and treat the abort as fallback.
🔧 Proposed fix
+const REQUEST_TIMEOUT_MS = 5000;
+
const res = await fetch(url, {
headers: { 'User-Agent': 'birdflop.com server list' },
+ signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
// Cloudflare-specific edge caching so repeated views are cheap and we
// stay friendly to mcstatus.io rate limits.
cf: { cacheTtl: CACHE_TTL_SECONDS, cacheEverything: true },
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const res = await fetch(url, { | |
| headers: { 'User-Agent': 'birdflop.com server list' }, | |
| // Cloudflare-specific edge caching so repeated views are cheap and we | |
| // stay friendly to mcstatus.io rate limits. | |
| cf: { cacheTtl: CACHE_TTL_SECONDS, cacheEverything: true }, | |
| }); | |
| if (!res.ok) return fallback; | |
| const data: McStatusResponse = await res.json(); | |
| try { | |
| const REQUEST_TIMEOUT_MS = 5000; | |
| const res = await fetch(url, { | |
| headers: { 'User-Agent': 'birdflop.com server list' }, | |
| signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), | |
| // Cloudflare-specific edge caching so repeated views are cheap and we | |
| // stay friendly to mcstatus.io rate limits. | |
| cf: { cacheTtl: CACHE_TTL_SECONDS, cacheEverything: true }, | |
| }); | |
| if (!res.ok) return fallback; | |
| const data: McStatusResponse = await res.json(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/serverlist/status.ts` around lines 57 - 67, Update the fetch call in
getServerStatus to pass an AbortSignal.timeout using the appropriate request
duration, and ensure an aborted request follows the existing fallback path.
Preserve the current response validation and McStatusResponse handling for
requests that complete normally.
| try { | ||
| const body = new FormData(); | ||
| body.append('secret', secret); | ||
| body.append('response', token); | ||
| if (remoteIp) body.append('remoteip', remoteIp); | ||
|
|
||
| const res = await fetch(VERIFY_URL, { method: 'POST', body }); | ||
| const data: TurnstileVerifyResponse = await res.json(); | ||
|
|
||
| if (data.success) return { success: true }; | ||
| return { | ||
| success: false, | ||
| error: `CAPTCHA verification failed${data['error-codes']?.length ? `: ${data['error-codes'].join(', ')}` : ''}`, | ||
| }; | ||
| } catch (err) { | ||
| console.error('Turnstile verification error:', err); | ||
| return { success: false, error: 'CAPTCHA verification error' }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching turnstile/votifier:"
fd -a 'turnstile\.ts|votifier\.ts' . | sed 's#^\./##'
echo
echo "Relevant snippets:"
for f in $(fd 'turnstile\.ts|votifier\.ts' .); do
echo "--- $f ($(wc -l < "$f") lines) ---"
sed -n '1,220p' "$f"
done
echo
echo "Search for withTimeout and turnstile verify:"
rg -n "withTimeout|verifyTurnstile|TurnstileVerifyResponse|VERIFY_URL|voteForServer|challenges.cloudflare.com|turnstile" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**'Repository: birdflop/web
Length of output: 11148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
targets = sorted(Path('.').glob('**/*.ts'))
turnstile_code = None
for path in targets:
text = path.read_text(errors='ignore')
if path.as_posix().endswith('src/util/serverlist/turnstile.ts'):
turnstile_code = text
continue
if not turnstile_code:
print("turnstile.ts not found")
raise SystemExit
checks = {
"fetch_without_signal": "const res = await fetch(VERIFY_URL, { method: 'POST', body });" in turnstile_code,
"fetch_has_signal": bool(re.search(r"fetch\(VERIFY_URL\s*,\s*{[\s\S]*?signal:", turnstile_code)),
"verify_returns_on_success": "if (data.success) return { success: true };" in turnstile_code,
"returns_success_false_on_failure": "return { success: false, error:" in turnstile_code,
}
for k, v in checks.items():
print(f"{k}={v}")
PYRepository: birdflop/web
Length of output: 268
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- src/util/serverlist/actions.ts lines 150-190 ---"
sed -n '150,190p' src/util/serverlist/actions.ts
echo
echo "--- package/dev tool clues ---"
for f in package.json vite.config.* qwik*.json .github/workflows/*; do
[ -f "$f" ] && echo "### $f" && sed -n '1,220p' "$f"
doneRepository: birdflop/web
Length of output: 16662
Add a timeout to the Turnstile verification fetch.
voteForServer awaits verifyTurnstile() on the server action request path. If challenges.cloudflare.com/turnstile/v0/siteverify does not respond, this fetch can hold the vote request until the outer platform timeout runs and no Votifier delivery is attempted.
src/util/serverlist/votifier.ts already defines a 5s timeout for external socket operations; use the same timeout discipline for this HTTP call, including AbortController.abort() plus clearing the timer.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/serverlist/turnstile.ts` around lines 27 - 44, Update the Turnstile
verification fetch in verifyTurnstile to use an AbortController with a 5-second
timeout, passing its signal to fetch and aborting when the timer expires. Clear
the timeout after the fetch completes or fails, while preserving the existing
success, failure, and catch handling.
| function parsePort( | ||
| value: string | number | undefined | ||
| ): number | null | undefined { | ||
| if (value === undefined || value === '' || value === null) return null; | ||
| const n = typeof value === 'number' ? value : parseInt(value, 10); | ||
| if (!Number.isInteger(n)) return undefined; // signal "invalid" | ||
| if (n < 1 || n > 65535) return undefined; | ||
| return n; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject port strings with trailing non-numeric characters.
Line 79 accepts 25565abc as 25565 and 1.5 as 1. validateServerInput is the persistence boundary, so malformed input is stored as a different port. Require the complete trimmed string to match decimal digits before converting it with Number().
Proposed fix
- if (value === undefined || value === '' || value === null) return null;
- const n = typeof value === 'number' ? value : parseInt(value, 10);
+ if (value === undefined || value === null) return null;
+ if (typeof value === 'string' && value.trim() === '') return null;
+ if (typeof value === 'string' && !/^\d+$/.test(value.trim()))
+ return undefined;
+ const n = typeof value === 'number' ? value : Number(value.trim());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function parsePort( | |
| value: string | number | undefined | |
| ): number | null | undefined { | |
| if (value === undefined || value === '' || value === null) return null; | |
| const n = typeof value === 'number' ? value : parseInt(value, 10); | |
| if (!Number.isInteger(n)) return undefined; // signal "invalid" | |
| if (n < 1 || n > 65535) return undefined; | |
| return n; | |
| function parsePort( | |
| value: string | number | undefined | |
| ): number | null | undefined { | |
| if (value === undefined || value === null) return null; | |
| if (typeof value === 'string' && value.trim() === '') return null; | |
| if (typeof value === 'string' && !/^\d+$/.test(value.trim())) | |
| return undefined; | |
| const n = typeof value === 'number' ? value : Number(value.trim()); | |
| if (!Number.isInteger(n)) return undefined; // signal "invalid" | |
| if (n < 1 || n > 65535) return undefined; | |
| return n; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/serverlist/validation.ts` around lines 75 - 82, Update parsePort to
validate string inputs after trimming, requiring the complete value to consist
only of decimal digits before conversion. Replace the partial parseInt behavior
with Number() after this check, while preserving existing undefined, empty,
null, integer, and 1–65535 range handling.
| // Hostnames: allow domain names and IPv4 (no scheme, no path). | ||
| const HOST_RE = /^[a-zA-Z0-9.-]+$/; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/util/serverlist/status.ts --items all
ast-grep outline src/util/serverlist/votifier.ts --items all
rg -n -C 3 --type ts \
'fetch\s*\(|https?\.request|net\.connect|createConnection|dgram|javaHost|bedrockHost|votifierHost' \
src/util/serverlist/status.ts \
src/util/serverlist/votifier.ts \
src/util/serverlist/actions.tsRepository: birdflop/web
Length of output: 3850
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)validation\.ts$|(^|/)status\.ts$|(^|/)votifier\.ts$|(^|/)actions\.ts$' || true
echo "== validation outline and relevant lines =="
sed -n '1,150p' src/util/serverlist/validation.ts
echo "== status.ts =="
cat -n src/util/serverlist/status.ts
echo "== votifier.ts =="
cat -n src/util/serverlist/votifier.ts
echo "== status/votifier imports and usages =="
rg -n -C 2 'validate.*host|host.*valid|validation|fetchStatus|sendVotifierV2|javaHost|bedrockHost|votifierHost|localhost|127\.0\.0\.1|private|reserved' src || trueRepository: birdflop/web
Length of output: 50368
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const HOST_RE = /^[a-zA-Z0-9.-]+$/;
const values = ['localhost', '127.0.0.1', '192.168.1.10', '10.0.0.1', '172.16.0.1', '169.254.1.1', '0.0.0.0'];
for (const v of values) {
console.log(v, HOST_RE.test(v));
}
try {
console.log('dns.lookup localhost', JSON.stringify(require('node:dns').lookupSync('localhost', { family: 0 }), null, 2));
} catch (e) {
console.log('dns.resolve localhost', JSON.stringify(require('node:dns').resolveSync('localhost'), null, 2));
}
JSRepository: birdflop/web
Length of output: 1018
Block private or reserved destinations before checking status or sending Votifier messages.
HOST_RE permits localhost, 127.0.0.1, private RFC 1918 ranges, link-local addresses, and 0.0.0.0. status.ts passes those values to fetch() for mcstatus, while sendVotifierV2() opens cloudflare:sockets using the stored Votifier host. Resolve hostnames before checking status, reject private/reserved destinations, and block reserved IPs before creating the Votifier TCP connection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/serverlist/validation.ts` around lines 94 - 95, Update server
destination validation around HOST_RE to resolve hostnames and reject private or
reserved IP ranges, including localhost, loopback, RFC 1918, link-local,
unspecified, and equivalent IPv6 addresses, before any status check or Votifier
message is sent. Ensure status.ts validates the resolved destination before
fetch() and sendVotifierV2() performs the same protection before creating the
cloudflare:sockets TCP connection.
| let socket: | ||
| | { | ||
| readable: ReadableStream; | ||
| writable: WritableStream; | ||
| close: () => Promise<void>; | ||
| } | ||
| | undefined; | ||
|
|
||
| try { | ||
| // Dynamically imported so this module never gets pulled into a client bundle. | ||
| const { connect } = (await import('cloudflare:sockets')) as { | ||
| connect: (addr: { hostname: string; port: number }) => { | ||
| readable: ReadableStream; | ||
| writable: WritableStream; | ||
| close: () => Promise<void>; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Type the socket streams to fix the no-unsafe-argument warnings.
socket.readable/socket.writable and the connect return type use bare ReadableStream/WritableStream, so reader.read() returns any for .value. That any value later flows into TextDecoder.decode() at Lines 108 and 142-144, which is what CI flags.
Type the streams as ReadableStream<Uint8Array>/WritableStream<Uint8Array> here, at the declaration sites, instead of casting at each call site.
🔧 Proposed fix
let socket:
| {
- readable: ReadableStream;
- writable: WritableStream;
+ readable: ReadableStream<Uint8Array>;
+ writable: WritableStream<Uint8Array>;
close: () => Promise<void>;
}
| undefined; const { connect } = (await import('cloudflare:sockets')) as {
connect: (addr: { hostname: string; port: number }) => {
- readable: ReadableStream;
- writable: WritableStream;
+ readable: ReadableStream<Uint8Array>;
+ writable: WritableStream<Uint8Array>;
close: () => Promise<void>;
};
};As per pipeline failures, "vp check reported TypeScript no-unsafe-argument: values of type any are passed to TextDecoder.decode."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let socket: | |
| | { | |
| readable: ReadableStream; | |
| writable: WritableStream; | |
| close: () => Promise<void>; | |
| } | |
| | undefined; | |
| try { | |
| // Dynamically imported so this module never gets pulled into a client bundle. | |
| const { connect } = (await import('cloudflare:sockets')) as { | |
| connect: (addr: { hostname: string; port: number }) => { | |
| readable: ReadableStream; | |
| writable: WritableStream; | |
| close: () => Promise<void>; | |
| }; | |
| }; | |
| let socket: | |
| | { | |
| readable: ReadableStream<Uint8Array>; | |
| writable: WritableStream<Uint8Array>; | |
| close: () => Promise<void>; | |
| } | |
| | undefined; | |
| try { | |
| // Dynamically imported so this module never gets pulled into a client bundle. | |
| const { connect } = (await import('cloudflare:sockets')) as { | |
| connect: (addr: { hostname: string; port: number }) => { | |
| readable: ReadableStream<Uint8Array>; | |
| writable: WritableStream<Uint8Array>; | |
| close: () => Promise<void>; | |
| }; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/serverlist/votifier.ts` around lines 75 - 91, Update the socket
declaration and the dynamically imported connect return type in the votifier
flow to use ReadableStream<Uint8Array> and WritableStream<Uint8Array> instead of
unparameterized stream types, ensuring reader.read().value is typed for
TextDecoder.decode without call-site casts.
Source: Pipeline failures
Auto-detect listings hosted on Birdflop by resolving their address (DNS-over-HTTPS, incl. Java SRV) against panel node/allocation IPs pulled from the Pterodactyl Application API and cached in D1 - no hardcoded IPs, nothing exposed to clients beyond a boolean badge. Re-checked on edit and lazily once a day on detail views. Also fixes issues found while auditing the feature: - strip Votifier host/port/token from public loader payloads (the token allowed forging votes straight to a server's NuVotifier port) - gate the edit loader so non-owners no longer receive the full row - return only name/image for listing owners instead of the whole user row (leaked emails) - require http(s) URLs for the Discord link (javascript: URI XSS) - set createdAt explicitly on inserts; the CURRENT_TIMESTAMP default stores text, making vote cooldowns permanent and monthly counts all-time (text sorts above numbers in SQLite) - cap listings at 10 per account, one report per server per user/IP per 24h, and guard against NaN page params
Deploying web with
|
| Latest commit: |
bddc927
|
| Status: | 🚫 Build failed. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.env.example:
- Line 11: Add a trailing blank line after the PANEL_URL entry in the
environment template so the file ends with the required blank line and satisfies
dotenv-linter’s EndingBlankLine check.
In `@drizzle/migrations/meta/0012_snapshot.json`:
- Line 1: Run vp check --fix to regenerate the formatting for the
0012_snapshot.json generated snapshot, then include the resulting file changes
in the commit.
In `@src/util/serverlist/birdflop.ts`:
- Around line 50-53: Update both dohQuery() and fetchPanelIps() to apply the
project’s existing abort/timeout policy to their external fetch calls, ensuring
neither request can remain unbounded. Preserve fetchPanelIps()’s stale IP-cache
fallback when the timed request fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 300fe797-29d6-4152-8e7f-98585e41f012
📒 Files selected for processing (14)
.env.exampledrizzle/migrations/0012_black_demogoblin.sqldrizzle/migrations/meta/0012_snapshot.jsondrizzle/migrations/meta/_journal.jsondrizzle/schema.tssrc/components/ServerList/ServerCard.tsxsrc/routes/serverlist/[slug]/edit/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/routes/serverlist/index.tsxsrc/util/serverlist/actions.tssrc/util/serverlist/birdflop.tssrc/util/serverlist/constants.tssrc/util/serverlist/queries.tssrc/util/serverlist/validation.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- src/util/serverlist/constants.ts
- src/routes/serverlist/[slug]/edit/index.tsx
- src/routes/serverlist/index.tsx
- src/util/serverlist/actions.ts
- src/util/serverlist/validation.ts
- src/routes/serverlist/[slug]/index.tsx
- src/components/ServerList/ServerCard.tsx
| # on the server list. Optional; detection is skipped when unset. | ||
| PANEL_API_KEY= | ||
| # Defaults to https://panel.birdflop.com when unset. | ||
| PANEL_URL= No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required ending blank line.
Add a blank line after PANEL_URL=. dotenv-linter reports EndingBlankLine for this file.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 11-11: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example at line 11, Add a trailing blank line after the PANEL_URL entry
in the environment template so the file ends with the required blank line and
satisfies dotenv-linter’s EndingBlankLine check.
Source: Linters/SAST tools
| const res = await fetch( | ||
| `${DOH_URL}?name=${encodeURIComponent(name)}&type=${type}`, | ||
| { headers: { accept: 'application/dns-json' } } | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## repository files matching birdflop.ts"
fd -a 'birdflop\.ts$' . || true
echo
echo "## outline"
ast-grep outline src/util/serverlist/birdflop.ts --view expanded || true
echo
echo "## relevant file sections"
wc -l src/util/serverlist/birdflop.ts
sed -n '1,230p' src/util/serverlist/birdflop.ts | cat -n
echo
echo "## fetch/timeout/policy references"
rg -n "fetch\\(|RequestTimeout|timeout|AbortSignal|AbortController|setTimeout|Promise.race|shared|policy|DOH_URL|apiKey|panelUrl" src || trueRepository: birdflop/web
Length of output: 22749
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## caller sites in birdflop.ts"
sed -n '204,350p' src/util/serverlist/birdflop.ts | cat -n
echo
echo "## actions usages"
sed -n '30,90p' src/util/serverlist/actions.ts | cat -n
rg -n "detectBirdflopHosted|refreshBirdflopHosted|isBirdflopHosted|birdflop" src/util/serverlist/actions.ts src/routes src/components -g '*.ts' -g '*.tsx' | head -n 120Repository: birdflop/web
Length of output: 18483
🌐 Web query:
Cloudflare Workers request limit time budget timeout abortController fetch documentation
💡 Result:
Cloudflare Workers support the standard Fetch API and AbortController/AbortSignal for managing request timeouts [1][2]. ### Fetch Timeouts with AbortController To implement a timeout for a fetch request in a Cloudflare Worker, you use an AbortController to signal the cancellation of the fetch operation [1][2]. Example implementation: javascript export default { async fetch(request) { const controller = new AbortController(); // Set a timeout of 5000 milliseconds const timeoutId = setTimeout(() => controller.abort(), 5000); try { const response = await fetch("https://example.com", { signal: controller.signal }); return response; } catch (err) { if (err.name === 'AbortError') { return new Response("Request timed out", { status: 504 }); } throw err; } finally { clearTimeout(timeoutId); } } }; ### Key Considerations 1. CPU Time vs. Wall Time: Workers have strict CPU time limits (e.g., 10ms for free tier, 30s-5m for paid tiers) [3][4]. Importantly, time spent waiting for network requests (including fetch calls) does not count toward CPU time [3]. 2. Wall Time Limits: While there is no hard "wall time" limit for incoming HTTP requests (as long as the client remains connected), Cron Triggers, Queue consumers, and Durable Object alarms have a 15-minute wall time limit [4]. 3. AbortSignal Support: The AbortSignal is used to cancel pending asynchronous operations [2][5]. When a signal passed to fetch is aborted, the fetch promise rejects with an AbortError [5]. 4. Context: Ensure you handle the fetch operation within your worker's execution context [1][6]. If you need to perform work after the response is sent, use ctx.waitUntil() [4]. For detailed limits, see the official Cloudflare Workers Limits documentation [3].
Citations:
- 1: https://developers.cloudflare.com/workers/runtime-apis/request/
- 2: https://developers.cloudflare.com/workers/runtime-apis/web-standards/index.md
- 3: https://developers.cloudflare.com/workers/platform/limits/
- 4: https://developers.cloudflare.com/workers/platform/limits/index.md
- 5: https://developers.cloudflare.com/workers/runtime-apis/scheduler/
- 6: https://developers.cloudflare.com/workers/runtime-apis/handlers/fetch/
Add deadlines to Birdflop external requests.
dohQuery() and fetchPanelIps() make unbounded Cloudflare fetch() calls, and these paths run during server creation plus detail-page refreshes. Add the same abort/timeout policy to both calls. The stale IP-cache fallback should remain for the panel refresh.
📍 Affects 1 file
src/util/serverlist/birdflop.ts#L50-L53(this comment)src/util/serverlist/birdflop.ts#L168-L176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/serverlist/birdflop.ts` around lines 50 - 53, Update both dohQuery()
and fetchPanelIps() to apply the project’s existing abort/timeout policy to
their external fetch calls, ensuring neither request can remain unbounded.
Preserve fetchPanelIps()’s stale IP-cache fallback when the timed request fails.
…into feat/add-serverlist
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/rgbirdflop/presets/PresetPreview.tsx`:
- Around line 48-55: Update getGradient to return undefined when colors is
absent or has length zero, before constructing the linear-gradient string.
Preserve the existing gradient generation for non-empty ColorStop arrays.
In `@src/routes/resources/rgb/presets/index.tsx`:
- Around line 442-445: The default perPage value of 20 is missing from both
pagination controls. Update the Pagination instances at
src/routes/resources/rgb/presets/index.tsx lines 442-445 and 474-477 to include
20 in their perPageOptions lists, keeping both controls consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1dccde48-8f54-41ef-89ca-3e3d44e3876e
📒 Files selected for processing (10)
drizzle/migrations/meta/0012_snapshot.jsondrizzle/migrations/meta/_journal.jsonsrc/components/Elements/MotdText.tsxsrc/components/Elements/Output.tsxsrc/components/Elements/Pagination.tsxsrc/components/ServerList/ServerCard.tsxsrc/components/rgbirdflop/presets/PresetPreview.tsxsrc/routes/resources/rgb/presets/index.tsxsrc/routes/serverlist/[slug]/index.tsxsrc/routes/serverlist/index.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- drizzle/migrations/meta/_journal.json
- src/components/Elements/Output.tsx
- drizzle/migrations/meta/0012_snapshot.json
- src/components/ServerList/ServerCard.tsx
- src/routes/serverlist/[slug]/index.tsx
- src/routes/serverlist/index.tsx
| export function getGradient(colors?: ColorStop[]) { | ||
| if (!colors) { | ||
| return undefined; | ||
| } | ||
| return `linear-gradient(to bottom right, ${colors | ||
| ?.map((color) => `${color.hex}10 ${color.pos}%`) | ||
| .join(', ')})`; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle empty color arrays.
ColorStop[] permits []. The current code then generates linear-gradient(to bottom right, ), which is invalid CSS. Return undefined when colors.length === 0.
Proposed fix
export function getGradient(colors?: ColorStop[]) {
- if (!colors) {
+ if (!colors?.length) {
return undefined;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function getGradient(colors?: ColorStop[]) { | |
| if (!colors) { | |
| return undefined; | |
| } | |
| return `linear-gradient(to bottom right, ${colors | |
| ?.map((color) => `${color.hex}10 ${color.pos}%`) | |
| .join(', ')})`; | |
| } | |
| export function getGradient(colors?: ColorStop[]) { | |
| if (!colors?.length) { | |
| return undefined; | |
| } | |
| return `linear-gradient(to bottom right, ${colors | |
| ?.map((color) => `${color.hex}10 ${color.pos}%`) | |
| .join(', ')})`; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/rgbirdflop/presets/PresetPreview.tsx` around lines 48 - 55,
Update getGradient to return undefined when colors is absent or has length zero,
before constructing the linear-gradient string. Preserve the existing gradient
generation for non-empty ColorStop arrays.
| totalCount={presetCount} | ||
| rowsLength={publicPresets.length} | ||
| itemLabel="presets" | ||
| perPageOptions={[12, 24, 48, 96]} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline src/components/Elements/Pagination.tsx --items all
rg -n -C 6 'perPageOptions|perPage|<select|option' \
src/components/Elements/Pagination.tsx \
src/routes/resources/rgb/presets/index.tsxRepository: birdflop/web
Length of output: 14266
Keep the default perPage selectable in both pagination controls.
perPage defaults to 20, but both <Pagination> instances pass [12, 24, 48, 96]. When no perPage URL parameter is present, the page renders 20 results while the per-page options do not include the active selection. Include 20 in both perPageOptions lists, or change the loader default to an option that is included.
📍 Affects 1 file
src/routes/resources/rgb/presets/index.tsx#L442-L445(this comment)src/routes/resources/rgb/presets/index.tsx#L474-L477
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/routes/resources/rgb/presets/index.tsx` around lines 442 - 445, The
default perPage value of 20 is missing from both pagination controls. Update the
Pagination instances at src/routes/resources/rgb/presets/index.tsx lines 442-445
and 474-477 to include 20 in their perPageOptions lists, keeping both controls
consistent.
Summary by CodeRabbit