-
Notifications
You must be signed in to change notification settings - Fork 5
Fix env variable grabbing #1512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,7 +9,7 @@ export async function loader({ params }: LoaderFunctionArgs) { | |||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||
const dapper = new DapperTs(() => { | ||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||
apiHost: import.meta.env.VITE_API_URL, | ||||||||||||||||||||||||||||||||
apiHost: process.env.VITE_API_URL, | ||||||||||||||||||||||||||||||||
sessionId: undefined, | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
Comment on lines
10
to
15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Validate presence of Ensure the server env is configured; otherwise return a 500 with a clear message. - const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const apiHost = process.env.VITE_API_URL;
+ if (!apiHost) {
+ throw new Response("API host is not configured (VITE_API_URL missing)", { status: 500 });
+ }
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,7 @@ export async function loader({ params }: LoaderFunctionArgs) { | |
try { | ||
const dapper = new DapperTs(() => { | ||
return { | ||
apiHost: import.meta.env.VITE_API_URL, | ||
apiHost: process.env.VITE_API_URL, | ||
sessionId: undefined, | ||
}; | ||
}); | ||
Comment on lines
10
to
15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainHarden env var usage: validate and normalize VITE_API_URL before passing to DapperTs process.env values are string | undefined. If VITE_API_URL is missing, DapperTs will receive undefined and fail later. Fail fast with a clear error and normalize trailing slashes to avoid accidental double slashes in requests. Apply this diff: - const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const rawApiHost = process.env.VITE_API_URL ?? "";
+ if (!rawApiHost) {
+ throw new Error("VITE_API_URL is not set in the server environment (SSR loader: Readme).");
+ }
+ const apiHost = rawApiHost.replace(/\/+$/, "");
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); Optional repo-wide verification to ensure no server loader still relies on import.meta.env for apiHost: 🏁 Script executed: #!/bin/bash
# Verify all server loaders use process.env for DapperTs apiHost
rg -nP --type=ts --type=tsx -C2 'new\s+DapperTs\s*\(\s*\(\)\s*=>\s*{\s*return\s*{\s*[^}]*apiHost:' apps/cyberstorm-remix | sed 's/^/HIT: /'
echo "---- Leftovers using import.meta.env.VITE_API_URL ----"
rg -n 'import\.meta\.env\.VITE_API_URL' apps/cyberstorm-remix || true Length of output: 669 Ensure all Your app still has a few places where • In • In Please refactor these to mirror the pattern in - import.meta.env.VITE_API_URL
+ (() => {
+ const rawApiHost = process.env.VITE_API_URL ?? "";
+ if (!rawApiHost) {
+ throw new Error(
+ "VITE_API_URL must be set in the environment (middleware/session loader)."
+ );
+ }
+ return rawApiHost.replace(/\/+$/, "");
+ })() For example, in - const session = getSessionContext(import.meta.env.VITE_API_URL);
+ const rawApiHost = process.env.VITE_API_URL ?? "";
+ if (!rawApiHost) {
+ throw new Error(
+ "VITE_API_URL is required for session context (middlewares.ts)."
+ );
+ }
+ const apiHost = rawApiHost.replace(/\/+$/, "");
+ const session = getSessionContext(apiHost); And update the client entry snippet similarly to ensure consistency across all loaders and entrypoints. 🤖 Prompt for AI Agents
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ export async function loader({ params }: LoaderFunctionArgs) { | |
try { | ||
const dapper = new DapperTs(() => { | ||
return { | ||
apiHost: import.meta.env.VITE_API_URL, | ||
apiHost: process.env.VITE_API_URL, | ||
sessionId: undefined, | ||
}; | ||
}); | ||
Comment on lines
33
to
38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainGuard against missing VITE_API_URL in SSR loader
- const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const apiHost = process.env.VITE_API_URL;
+ if (!apiHost) {
+ throw new Response("API host is not configured (VITE_API_URL missing)", { status: 500 });
+ }
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); To scan the repo for other unguarded usages and review them quickly, run: 🏁 Script executed: #!/bin/bash
# Find all SSR loaders using process.env.VITE_API_URL without a guard nearby
rg -n --type=ts --type=tsx 'process\.env\.VITE_API_URL' -C2 Length of output: 89 🏁 Script executed: #!/bin/bash
# Find all occurrences of VITE_API_URL in TS/TSX files with context
rg -n 'process\.env\.VITE_API_URL' -g '*.ts' -g '*.tsx' -C2 Length of output: 6218 Guard against missing VITE_API_URL in all SSR loaders We found 15 unguarded initializations of • Files (unguarded
Suggested essential refactor
export function getApiHost(): string {
const host = process.env.VITE_API_URL;
if (!host) {
throw new Response(
"API host is not configured (VITE_API_URL missing)",
{ status: 500 }
);
}
return host;
}
-import { DapperTs } from "dapper-ts";
-const dapper = new DapperTs(() => ({
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
-}));
+import { DapperTs } from "dapper-ts";
+import { getApiHost } from "~/utils/getApiHost";
+const dapper = new DapperTs(() => ({
+ apiHost: getApiHost(),
+ sessionId: undefined,
+})); This ensures every SSR loader fails fast with a clear 500 if the API URL isn’t set. 🤖 Prompt for AI Agents
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -10,7 +10,7 @@ export async function loader({ params }: LoaderFunctionArgs) { | |||||||||||||||||||||||||||||||
if (params.communityId && params.namespaceId && params.packageId) { | ||||||||||||||||||||||||||||||||
const dapper = new DapperTs(() => { | ||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||
apiHost: import.meta.env.VITE_API_URL, | ||||||||||||||||||||||||||||||||
apiHost: process.env.VITE_API_URL, | ||||||||||||||||||||||||||||||||
sessionId: undefined, | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||
Comment on lines
11
to
16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add an SSR env invariant for Keep the pattern consistent and fail fast if the host is not configured. - const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const apiHost = process.env.VITE_API_URL;
+ if (!apiHost) {
+ throw new Response("API host is not configured (VITE_API_URL missing)", { status: 500 });
+ }
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -15,7 +15,7 @@ export async function loader({ params }: LoaderFunctionArgs) { | |||||||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||||||||
const dapper = new DapperTs(() => { | ||||||||||||||||||||||||||||||||||||||
return { | ||||||||||||||||||||||||||||||||||||||
apiHost: import.meta.env.VITE_API_URL, | ||||||||||||||||||||||||||||||||||||||
apiHost: process.env.VITE_API_URL, | ||||||||||||||||||||||||||||||||||||||
sessionId: undefined, | ||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||
Comment on lines
16
to
21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion SSR env guard for Same as other loaders; add a clear failure mode when env is missing. - const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const apiHost = process.env.VITE_API_URL;
+ if (!apiHost) {
+ throw new Response("API host is not configured (VITE_API_URL missing)", { status: 500 });
+ }
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ export async function loader({ request, params }: LoaderFunctionArgs) { | |
try { | ||
const dapper = new DapperTs(() => { | ||
return { | ||
apiHost: import.meta.env.VITE_API_URL, | ||
apiHost: process.env.VITE_API_URL, | ||
sessionId: undefined, | ||
}; | ||
}); | ||
Comment on lines
23
to
28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent ❓ Verification inconclusiveSSR env may be undefined — validate Same reasoning as other loaders; fail fast with a clear error if env is missing. - const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const apiHost = process.env.VITE_API_URL;
+ if (!apiHost) {
+ throw new Response("API host is not configured (VITE_API_URL missing)", { status: 500 });
+ }
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); Validate VITE-prefixed env variables are only exposed as strings via • File: Please update this block to extract and validate the API host, failing fast with a clear 500 error if it’s missing: - const dapper = new DapperTs(() => {
- return {
- apiHost: process.env.VITE_API_URL,
- sessionId: undefined,
- };
- });
+ const apiHost = process.env.VITE_API_URL;
+ if (!apiHost) {
+ throw new Response(
+ "API host is not configured (VITE_API_URL missing)",
+ { status: 500 }
+ );
+ }
+ const dapper = new DapperTs(() => ({
+ apiHost,
+ sessionId: undefined,
+ })); 🤖 Prompt for AI Agents
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,4 @@ | ||
export const API_DOMAIN = | ||
process.env.NEXT_PUBLIC_API_URL || "https://thunderstore.io"; | ||
export const API_DOMAIN = process.env.VITE_API_URL || "https://thunderstore.io"; | ||
|
||
export const ROOT_DOMAIN = | ||
process.env.NEXT_PUBLIC_SITE_URL || "https://thunderstore.io"; | ||
process.env.VITE_SITE_URL || "https://thunderstore.io"; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -137,8 +137,10 @@ def setup_frontend(): | |||||||||||||||||||||
|
||||||||||||||||||||||
def start_frontend() -> BgProcess: | ||||||||||||||||||||||
configs = { | ||||||||||||||||||||||
"PUBLIC_SITE_URL": "http://127.0.0.1:8000", | ||||||||||||||||||||||
"PUBLIC_API_URL": "http://127.0.0.1:8000", | ||||||||||||||||||||||
"VITE_SITE_URL": "http://127.0.0.1:8000", | ||||||||||||||||||||||
"VITE_API_URL": "http://127.0.0.1:8000", | ||||||||||||||||||||||
"VITE_AUTH_BASE_URL": "http://127.0.0.1:8000", | ||||||||||||||||||||||
"VITE_AUTH_RETURN_URL": "http://127.0.0.1:8000", | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+140
to
144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify SITE/RETURN URL ports (likely 3000) vs API/AUTH_BASE (8000) to avoid redirect/domain mismatches The dev server is started on port 3000 (Line 146), but VITE_SITE_URL and VITE_AUTH_RETURN_URL are set to 8000. If auth flows construct return URLs using these, the browser may be redirected to 8000 instead of the frontend (3000). Consider mapping:
Apply if appropriate: - "VITE_SITE_URL": "http://127.0.0.1:8000",
- "VITE_API_URL": "http://127.0.0.1:8000",
- "VITE_AUTH_BASE_URL": "http://127.0.0.1:8000",
- "VITE_AUTH_RETURN_URL": "http://127.0.0.1:8000",
+ "VITE_SITE_URL": "http://127.0.0.1:3000",
+ "VITE_API_URL": "http://127.0.0.1:8000",
+ "VITE_AUTH_BASE_URL": "http://127.0.0.1:8000",
+ "VITE_AUTH_RETURN_URL": "http://127.0.0.1:3000", To quickly confirm port usage and return URL construction hotspots: 🏁 Script executed: #!/bin/bash
# Frontend dev server port
rg -nC2 'workspace", "@thunderstore/cyberstorm-remix", "start".*--port", "3000' tools/visual-diff-ci/run_ci_script.py
# Places building login/return URLs or using these envs
rg -n --type=ts --type=tsx 'VITE_(SITE_URL|AUTH_RETURN_URL|AUTH_BASE_URL|API_URL)' Length of output: 443 🏁 Script executed: #!/bin/bash
# Confirm frontend dev server port usage
rg -nC2 '--port", "3000' tools/visual-diff-ci/run_ci_script.py
# Show the environment configuration block
rg -n 'configs\s*=\s*{' -C5 tools/visual-diff-ci/run_ci_script.py Length of output: 643 Fix frontend URL ports to match the dev server The CI script launches the frontend on port 3000 (BgProcess at line 146), but both • File: tools/visual-diff-ci/run_ci_script.py - "VITE_SITE_URL": "http://127.0.0.1:8000",
- "VITE_AUTH_RETURN_URL": "http://127.0.0.1:8000",
+ "VITE_SITE_URL": "http://127.0.0.1:3000",
+ "VITE_AUTH_RETURN_URL": "http://127.0.0.1:3000", Leave the API-related URLs on port 8000: "VITE_API_URL": "http://127.0.0.1:8000",
"VITE_AUTH_BASE_URL": "http://127.0.0.1:8000", 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
|
||||||||||||||||||||||
process = BgProcess( | ||||||||||||||||||||||
[YARN_PATH, "workspace", "@thunderstore/cyberstorm-remix", "start", "--host", "0.0.0.0", "--port", "3000"], | ||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
SSR env switch is correct; add a fail-fast guard for missing VITE_API_URL
Good move to
process.env
for SSR. IfVITE_API_URL
is unset in the server env, DapperTs calls will fail later with harder-to-debug errors. Fail fast with a clear 500.Apply this diff:
Verification helper to catch any remaining SSR usages of
import.meta.env
and spot loaders missing the guard:🏁 Script executed:
Length of output: 4801
🏁 Script executed:
Length of output: 47761
Add fail-fast guard for missing VITE_API_URL across all SSR loaders
When
apiHost
is sourced directly fromprocess.env.VITE_API_URL
, a missing env var will only surface as obscure downstream errors. Failing fast with a clear 500 prevents hard-to-debug issues.Apply this diff (or extract into a shared
getServerConfig()
helper) at each loader/DapperTs initialization that currently usesprocess.env.VITE_API_URL
:Locations requiring this update:
🤖 Prompt for AI Agents