Skip to content

fix: retry electric proxy and delay key toast #63

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.

Sign up for GitHub

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

Merged
1 commit merged into from
Oct 19, 2025
Merged

fix: retry electric proxy and delay key toast #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 56 additions & 25 deletions apps/server/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,46 +317,34 @@ new Elysia()
if (value !== null) passthroughParams.set(key, value);
}

const target = new URL(`${ELECTRIC_BASE_URL}/v1/shape`);
passthroughParams.forEach((value, key) => {
target.searchParams.set(key, value);
});

let allowTables = DEFAULT_GATEKEEPER_TABLES.slice();
let chatIdParam: string | null = null;
switch (scope) {
case "chats": {
target.searchParams.set("table", "chat");
target.searchParams.set("where", `"user_id" = $1`);
target.searchParams.set("params[1]", userId);
target.searchParams.set("columns", "id,title,updated_at,last_message_at,user_id");
allowTables = ["chat"];
break;
}
case "messages": {
const chatId = url.searchParams.get("chatId");
if (!chatId) {
chatIdParam = url.searchParams.get("chatId");
if (!chatIdParam) {
return withSecurityHeaders(new Response("Missing chatId", { status: 400 }), context.request);
}
try {
const owned = await db
.select({ id: chat.id })
.from(chat)
.where(and(eq(chat.id, chatId), eq(chat.userId, userId)));
.where(and(eq(chat.id, chatIdParam), eq(chat.userId, userId)));
if (owned.length === 0) {
if (!inMemoryChatOwned(userId, chatId)) {
if (!inMemoryChatOwned(userId, chatIdParam)) {
return withSecurityHeaders(new Response("Not Found", { status: 404 }), context.request);
}
}
} catch (error) {
if (process.env.NODE_ENV !== "test") console.error("chat.verify", error);
if (!inMemoryChatOwned(userId, chatId)) {
if (!inMemoryChatOwned(userId, chatIdParam)) {
return withSecurityHeaders(new Response("Not Found", { status: 404 }), context.request);
}
}
target.searchParams.set("table", "message");
target.searchParams.set("where", `"chat_id" = $1`);
target.searchParams.set("params[1]", chatId);
target.searchParams.set("columns", "id,chat_id,role,content,created_at,updated_at");
allowTables = ["message"];
break;
}
Expand All @@ -377,14 +365,57 @@ new Elysia()
const ifNoneMatch = context.request.headers.get("if-none-match");
if (ifNoneMatch) upstreamHeaders.set("if-none-match", ifNoneMatch);

let upstreamResponse: Response;
try {
upstreamResponse = await fetch(target, {
method: "GET",
headers: upstreamHeaders,
const baseCandidates = [ELECTRIC_BASE_URL];
const baseWithoutPort = ELECTRIC_BASE_URL.replace(/:\\d+$/, "");
const candidate3000 = `${baseWithoutPort}:3000`;
if (!baseCandidates.includes(candidate3000)) {
baseCandidates.push(candidate3000);
}
Comment on lines +368 to +373
Copy link
Contributor

@greptile-apps greptile-apps bot Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: The fallback logic adds :3000 to the base URL even if it already has a different port. If ELECTRIC_BASE_URL is http://example.com:3010, this creates http://example.com:3000. Is this the intended behavior, or should it skip the fallback when a port is explicitly configured? Should the fallback only apply when no explicit port is configured in ELECTRIC_BASE_URL?


const buildTarget = (base: string) => {
const target = new URL(`${base}/v1/shape`);
passthroughParams.forEach((value, key) => {
target.searchParams.set(key, value);
});
} catch (error) {
console.error("electric.fetch", error);
switch (scope) {
case "chats":
target.searchParams.set("table", "chat");
target.searchParams.set("where", `"user_id" = $1`);
target.searchParams.set("params[1]", userId);
target.searchParams.set("columns", "id,title,updated_at,last_message_at,user_id");
break;
case "messages": {
target.searchParams.set("table", "message");
target.searchParams.set("where", `"chat_id" = $1`);
target.searchParams.set("params[1]", chatIdParam ?? "");
target.searchParams.set("columns", "id,chat_id,role,content,created_at,updated_at");
break;
}
}
return target;
};

let upstreamResponse: Response | null = null;
let lastError: unknown = null;
for (const base of baseCandidates) {
const target = buildTarget(base);
try {
const response = await fetch(target, {
method: "GET",
headers: upstreamHeaders,
});
if (response.status < 500) {
upstreamResponse = response;
break;
}
lastError = new Error(`electric responded ${response.status}`);
Comment on lines +407 to +411
Copy link
Contributor

@greptile-apps greptile-apps bot Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Treats any 5xx response as a failure and retries the next candidate. This means a legitimate503 from Electric will trigger a retry to :3000, which may produce a confusing error if :3000 is also unavailable or returns a different 5xx. Consider whether 5xx responses should be returned immediately or if only connection errors should trigger retry. Should server errors (5xx) from Electric trigger a retry, or only network/connection failures?

} catch (error) {
lastError = error;
}
}

if (!upstreamResponse) {
console.error("electric.fetch", lastError);
return withSecurityHeaders(new Response("Electric service unreachable", { status: 504 }), context.request);
}

Expand Down
5 changes: 4 additions & 1 deletion apps/web/src/components/chat-room.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export default function ChatRoom({ chatId, initialMessages }: ChatRoomProps) {
const [modelsLoading, setModelsLoading] = useState(false);
const [modelOptions, setModelOptions] = useState<{ value: string; label: string; description?: string }[]>([]);
const [selectedModel, setSelectedModel] = useState<string | null>(null);
const [checkedApiKey, setCheckedApiKey] = useState(false);
const missingKeyToastRef = useRef<string | number | null>(null);

useEffect(() => {
Expand Down Expand Up @@ -158,10 +159,12 @@ export default function ChatRoom({ chatId, initialMessages }: ChatRoomProps) {
setApiKey(stored);
await fetchModels(stored);
}
setCheckedApiKey(true);
})();
}, [fetchModels]);

useEffect(() => {
if (!checkedApiKey) return;
Copy link
Contributor

@greptile-apps greptile-apps bot Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: Early return prevents toast from ever showing – the checkedApiKey flag is set to true (line 162) before the conditional check, so this guard will never trigger on the initial mount when apiKey is null. Did you intend to set checkedApiKey after the API key check completes, or should the guard check happen after state updates?

if (!apiKey) {
if (missingKeyToastRef.current == null) {
missingKeyToastRef.current = toast.warning("Add your OpenRouter API key", {
Expand All @@ -177,7 +180,7 @@ export default function ChatRoom({ chatId, initialMessages }: ChatRoomProps) {
toast.dismiss(missingKeyToastRef.current);
missingKeyToastRef.current = null;
}
}, [apiKey, router]);
}, [apiKey, router, checkedApiKey]);

const handleSaveApiKey = useCallback(
async (key: string) => {
Expand Down