Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions src/Projects.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as React from "react";
import { ProjectData, SERVER } from "./api";
import { ProjectData, ProjectsRequestTimeoutError, SERVER } from "./api";

export interface NameById {
[key: number]: string;
Expand All @@ -22,7 +22,11 @@ export default function Projects({ selectedUser, nameById }: ProjectsProps) {
setProjects(projects => [...(projects ?? []), ...page.projects]);
}
setHasMoreResults(page.hasMoreResults);
}).catch(() => {
}).catch((error) => {
if (error instanceof ProjectsRequestTimeoutError) {
alert(error.message);
return;
}
alert("Something went wrong...");
});
}, [selectedUser]);
Expand All @@ -47,4 +51,4 @@ export default function Projects({ selectedUser, nameById }: ProjectsProps) {
<button disabled={!hasMoreResults} onClick={loadMore}>{projects?.length ? `${projects.length} Project(s) Loaded - ` : ''}Load more</button>
</div>
);
}
}
37 changes: 35 additions & 2 deletions src/api/apiImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,27 @@ export interface ProjectsResponse {
hasMoreResults: boolean;
}

const PROJECTS_REQUEST_TIMEOUT_MS = 1000;

export const PROJECTS_REQUEST_TIMEOUT_MESSAGE =
"Request timed out. Possible infinite loop in api/pagination.py.";

export class ProjectsRequestTimeoutError extends Error {
constructor(message: string = PROJECTS_REQUEST_TIMEOUT_MESSAGE) {
super(message);
this.name = "ProjectsRequestTimeoutError";
}
}

function isAbortError(error: unknown): boolean {
return (
typeof error === "object" &&
error !== null &&
"name" in error &&
error.name === "AbortError"
);
}

class DefaultServer {
async getUsers(): Promise<UserData[]> {
const response = await fetch('http://127.0.0.1:5000/api/users');
Expand All @@ -28,8 +49,20 @@ class DefaultServer {
url.searchParams.append('pageSize', options.pageSize.toString());
}

const response = await fetch(url);
return response.json();
const controller = new AbortController();
const timeoutId = window.setTimeout(() => controller.abort(), PROJECTS_REQUEST_TIMEOUT_MS);

try {
const response = await fetch(url, { signal: controller.signal });
return response.json();
Copy link

Choose a reason for hiding this comment

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

Missing await makes timeout not cover body reading

Medium Severity

return response.json() without await inside the try/catch/finally causes the finally block to run (clearing the timeout via clearTimeout) immediately after headers arrive, before the response body is fully read. This means the abort timeout only protects the initial fetch, not the body reading phase. Additionally, if response.json() rejects, the catch block is bypassed entirely, so an AbortError during body reading would never be converted to a ProjectsRequestTimeoutError. Using return await response.json() would fix both issues.

Additional Locations (1)
Fix in Cursor Fix in Web

} catch (error) {
if (isAbortError(error)) {
throw new ProjectsRequestTimeoutError();
}
throw error;
} finally {
window.clearTimeout(timeoutId);
}
}
}

Expand Down
36 changes: 23 additions & 13 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { render } from "react-dom";

import App from "./App";
import { PROJECTS, USERS } from "./api/data";
import { ProjectData, ProjectsResponse, SERVER, UserData } from "./api";
import { ProjectData, ProjectsRequestTimeoutError, ProjectsResponse, SERVER, UserData } from "./api";

const rootElement = document.getElementById("root");
render(<App />, rootElement);
Expand All @@ -16,18 +16,26 @@ render(<App />, rootElement);
let hasMoreResults = true;
let lastProject: ProjectData | undefined = undefined;

while (hasMoreResults) {
const page: ProjectsResponse = await SERVER.getProjects({ pageSize, startAfter: lastProject, userId: user?.id?.toString() });
if (page.hasMoreResults && page.projects.length < pageSize) {
console.log(
`❌ ${userString} // Improperly sized page - hasMoreResults: true but results.length < pageSize`
);
console.groupEnd();
try {
while (hasMoreResults) {
const page: ProjectsResponse = await SERVER.getProjects({ pageSize, startAfter: lastProject, userId: user?.id?.toString() });
if (page.hasMoreResults && page.projects.length < pageSize) {
console.log(
`❌ ${userString} // Improperly sized page - hasMoreResults: true but results.length < pageSize`
);
console.groupEnd();
return;
}
totalCountFromApi += page.projects.length;
hasMoreResults = page.hasMoreResults;
lastProject = page.projects[page.projects.length - 1];
}
} catch (error) {
if (error instanceof ProjectsRequestTimeoutError) {
console.log(`❌ ${userString} // ${error.message}`);
return;
}
totalCountFromApi += page.projects.length;
hasMoreResults = page.hasMoreResults;
lastProject = page.projects[page.projects.length - 1];
throw error;
}

const filterCallback = user ? (project: ProjectData) => project.creatorId === user.id : undefined;
Expand All @@ -39,5 +47,7 @@ render(<App />, rootElement);
console.log(`✅ ${userString} // Counts match: ${totalCountFromApi}`);
}
}
Promise.all([...USERS.map(testUser), testUser()]);
}
Promise.all([...USERS.map(testUser), testUser()]).catch((error) => {
console.error(error);
});
}
Loading