Skip to content
Merged
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
113 changes: 52 additions & 61 deletions src/components/ActivityFeed.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const isEventTypeArray = (data: unknown): data is EventType[] => {
export default function ActivityFeed({ username }: { username: string }) {
const [events, setEvents] = useState<EventType[]>([]);
const [loading, setLoading] = useState(true);
const [error, setError] = useState<FetchError | null>(null);
const [error, setError] = useState("");

// 🕒 time ago function
const getTimeAgo = (dateString: string) => {
Expand All @@ -76,69 +76,53 @@ export default function ActivityFeed({ username }: { username: string }) {

useEffect(() => {
const fetchEvents = async () => {
try {
setLoading(true);
setError(null);

const res = await fetch(
`https://api.github.com/users/${username}/events`
);

// ✅ Check HTTP response success
if (!res.ok) {
const isRateLimited = res.status === 403;
const errorData = await res.json().catch(() => ({}));
const errorMessage =
typeof errorData === "object" &&
errorData !== null &&
"message" in errorData &&
typeof errorData.message === "string"
? errorData.message
: `HTTP ${res.status}: Failed to fetch events`;

throw new ActivityFeedError(
isRateLimited
? "GitHub API rate limit exceeded. Try again later."
: errorMessage,
isRateLimited,
res.status
try {
setLoading(true);
setError("");

const res = await fetch(
`https://api.github.com/users/${username}/events`
);
}

const data = await res.json();
// Handle GitHub API rate limit
if (res.status === 403) {
const remaining =
res.headers.get("X-RateLimit-Remaining") || "0";

// ✅ Validate response structure matches EventType[]
if (!isEventTypeArray(data)) {
throw new ActivityFeedError(
"Invalid API response structure. Expected array of events.",
false,
200
);
}
const reset =
res.headers.get("X-RateLimit-Reset");

setEvents(data);
} catch (err) {
const fetchError: FetchError = {
message: "Failed to fetch activity. Please try again.",
isRateLimited: false,
};

// Handle ActivityFeedError instances
if (err instanceof ActivityFeedError) {
fetchError.message = err.message;
fetchError.isRateLimited = err.isRateLimited;
fetchError.statusCode = err.statusCode;
} else if (err instanceof Error) {
// Handle generic errors
fetchError.message = err.message || fetchError.message;
}
const resetTime = reset
? new Date(Number(reset) * 1000).toLocaleTimeString()
: "Unknown";

setError(fetchError);
console.error("ActivityFeed fetch error:", fetchError);
setEvents([]);
} finally {
setLoading(false);
}
setError(
`GitHub API rate limit exceeded.
Please try again after ${resetTime}.
Remaining Requests: ${remaining}`
);
Comment on lines +99 to +103
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Error string formatting includes unintended whitespace/newlines.

The multiline template literal will render with extra spacing. Use a single-line template string for cleaner alert text.

🤖 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/ActivityFeed.tsx` around lines 51 - 55, The error message
passed to setError in ActivityFeed.tsx uses a multiline template literal which
introduces unintended whitespace and newlines; replace it with a single-line
interpolated string (e.g. `GitHub API rate limit exceeded. Please try again
after ${resetTime}. Remaining Requests: ${remaining}`) or build the message via
concatenation/trim to eliminate extra spacing before calling setError; update
the invocation where setError(...) is used so the alert text is a compact
single-line string.


setEvents([]);
setLoading(false);
return;
}
Comment on lines +88 to +108
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the relevant section with context
sed -n '1,220p' src/components/ActivityFeed.tsx | nl -ba | sed -n '1,220p' | cat

# Narrow search for fetch/useEffect and error handling
rg -n "useEffect|fetch\\(|res\\.status|403|X-RateLimit-Remaining|X-RateLimit-Reset|setError\\(" src/components/ActivityFeed.tsx

# Print a tighter window around the snippet area
sed -n '1,140p' src/components/ActivityFeed.tsx | nl -ba | sed -n '20,120p' | cat

Repository: GitMetricsLab/github_tracker

Length of output: 115


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Basic sanity: file exists?
ls -la src/components/ActivityFeed.tsx

# Show the file with line numbers (full file may be small; otherwise window it)
wc -l src/components/ActivityFeed.tsx

# Print the likely region
sed -n '1,220p' src/components/ActivityFeed.tsx | cat -n

# Focused grep for relevant patterns
rg -n "useEffect|fetch\\(|res\\.status|\\b403\\b|X-RateLimit-Remaining|X-RateLimit-Reset|setError\\(" src/components/ActivityFeed.tsx

# Print a tighter window around the 403 handling by searching for the snippet
line=$(rg -n "\\bres\\.status\\s*===\\s*403\\b" src/components/ActivityFeed.tsx | cut -d: -f1 | head -n1 || true)
if [ -n "${line}" ]; then
  start=$((line-30))
  end=$((line+60))
  if [ "$start" -lt 1 ]; then start=1; fi
  sed -n "${start},${end}p" src/components/ActivityFeed.tsx | cat -n
fi

# Check if there's any abort controller or cancellation logic
rg -n "AbortController|abort\\(|signal\\b|cleanup\\b|return \\(\\) =>|didCancel|isMounted|requestId" src/components/ActivityFeed.tsx

Repository: GitMetricsLab/github_tracker

Length of output: 8251


Fix incorrect 403 classification and stale fetch state in ActivityFeed

  • res.status === 403 is currently treated as a GitHub rate-limit error unconditionally (and X-RateLimit-Remaining defaults to "0" if missing), which will mislabel other 403 responses. Gate the rate-limit message on rate-limit headers instead (e.g., X-RateLimit-Remaining === "0"), otherwise show a generic forbidden/error.
Suggested fix
-          if (res.status === 403) {
-            const remaining =
-              res.headers.get("X-RateLimit-Remaining") || "0";
+          if (res.status === 403) {
+            const remaining = res.headers.get("X-RateLimit-Remaining");
             const reset =
               res.headers.get("X-RateLimit-Reset");
+            const isRateLimited = remaining === "0";
 
-            const resetTime = reset
-              ? new Date(Number(reset) * 1000).toLocaleTimeString()
-              : "Unknown";
-
-            setError(
-              `GitHub API rate limit exceeded.
-               Please try again after ${resetTime}.
-               Remaining Requests: ${remaining}`
-            );
+            if (isRateLimited) {
+              const resetTime = reset
+                ? new Date(Number(reset) * 1000).toLocaleTimeString()
+                : "Unknown";
+              setError(
+                `GitHub API rate limit exceeded. Please try again after ${resetTime}. Remaining Requests: ${remaining}`
+              );
+            } else {
+              setError("Access forbidden while fetching GitHub activity.");
+            }
 
             setEvents([]);
             setLoading(false);
             return;
           }
  • useEffect uses setInterval(fetchEvents, 30000) and triggers fetchEvents() again without any request cancellation/in-flight guard; overlapping or in-flight responses can overwrite newer state when username changes or when calls race. Add an AbortController/signal and/or a request-id guard before calling setEvents/setError/setLoading.

  • Minor: the rate-limit message is a multiline template literal; simplify it to a single-line string to avoid awkward whitespace in the rendered text.

📝 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.

Suggested change
if (res.status === 403) {
const remaining =
res.headers.get("X-RateLimit-Remaining") || "0";
const reset =
res.headers.get("X-RateLimit-Reset");
const resetTime = reset
? new Date(Number(reset) * 1000).toLocaleTimeString()
: "Unknown";
setError(
`GitHub API rate limit exceeded.
Please try again after ${resetTime}.
Remaining Requests: ${remaining}`
);
setEvents([]);
setLoading(false);
return;
}
if (res.status === 403) {
const remaining = res.headers.get("X-RateLimit-Remaining");
const reset =
res.headers.get("X-RateLimit-Reset");
const isRateLimited = remaining === "0";
if (isRateLimited) {
const resetTime = reset
? new Date(Number(reset) * 1000).toLocaleTimeString()
: "Unknown";
setError(
`GitHub API rate limit exceeded. Please try again after ${resetTime}. Remaining Requests: ${remaining}`
);
} else {
setError("Access forbidden while fetching GitHub activity.");
}
setEvents([]);
setLoading(false);
return;
}
🤖 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/ActivityFeed.tsx` around lines 40 - 60, The code treats any
403 as a rate-limit and can let stale responses overwrite state; update the
fetchEvents logic in ActivityFeed to: 1) detect rate-limit only when rate-limit
headers indicate it (e.g., const remaining =
res.headers.get("X-RateLimit-Remaining"); if (remaining === "0") { const reset =
res.headers.get("X-RateLimit-Reset"); const resetTime = reset ? new
Date(Number(reset) * 1000).toLocaleTimeString() : "Unknown"; setError(`GitHub
API rate limit exceeded. Please try again after ${resetTime}. Remaining
Requests: ${remaining}`); } else if (res.status === 403) { setError("Access
forbidden"); } 2) introduce an AbortController and/or a monotonically increasing
requestId inside useEffect around fetchEvents, pass signal to fetch, abort on
cleanup, and ignore responses whose requestId is stale before calling
setEvents/setError/setLoading; and 3) make the rate-limit message a single-line
string to avoid whitespace issues.


if (!res.ok) {
throw new Error("Failed to fetch activity");
}

const data = await res.json();

setEvents(data);
} catch (err) {
console.error(err);

setError(
"Something went wrong while fetching GitHub activity."
);
} finally {
setLoading(false);
}
Comment on lines 78 to +125
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ls -la
echo "---- ActivityFeed.tsx (lines 1-200) ----"
sed -n '1,200p' src/components/ActivityFeed.tsx
echo "---- ActivityFeed.tsx (lines 200-400) ----"
sed -n '200,400p' src/components/ActivityFeed.tsx

Repository: GitMetricsLab/github_tracker

Length of output: 5532


Guard against stale/out-of-order GitHub fetches in ActivityFeed.
fetchEvents runs on mount and every 30s, but the cleanup only clears the interval; in-flight fetch calls aren’t canceled. When username changes or the component unmounts, earlier responses can still call setEvents/setError/setLoading(false), overwriting newer state. Add an AbortController (or request-id guard) canceled in cleanup, and skip all state updates (including setLoading(false) in finally) for aborted/outdated requests.

🤖 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/ActivityFeed.tsx` around lines 30 - 77, fetchEvents can
produce out-of-order state updates because in-flight fetches aren't canceled;
update fetchEvents to create an AbortController per invocation (or a monotonic
requestId) and pass its signal to fetch, store the controller/requestId in the
surrounding useEffect scope, and cancel it in the useEffect cleanup (along with
clearInterval). In fetchEvents, detect aborted requests (e.g., err.name ===
'AbortError' or controller.signal.aborted / mismatched requestId) and return
early so you do not call setEvents, setError, or setLoading(false) for
aborted/outdated requests; ensure the finally block also skips setLoading when
the request was aborted. Use the function name fetchEvents and state setters
setEvents, setError, setLoading to locate the changes.

};

fetchEvents();
Expand All @@ -149,9 +133,16 @@ export default function ActivityFeed({ username }: { username: string }) {

return (
<div className="p-4">
<h2 className="text-xl font-bold mb-4 text-center">
<h2 className="text-xl font-bold mb-4 text-center text-black dark:text-white">
Activity Feed
</h2>

{error && (
<div className="bg-yellow-100 dark:bg-yellow-900 border border-yellow-400 text-yellow-800 dark:text-yellow-200 px-4 py-3 rounded mb-4">
{error}
</div>
)}


{loading ? (
<p className="text-center text-gray-500">Loading activity...</p>
Expand All @@ -172,7 +163,7 @@ export default function ActivityFeed({ username }: { username: string }) {
)}
</div>
) : events.length === 0 ? (
<p className="text-center text-gray-500">No activity found</p>
<p className="text-center text-black dark:text-white">No activity found</p>
) : (
events.slice(0, 10).map((event) => (
<div
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Activity.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ export default function Activity() {
return (
<div className="w-full h-full p-6 bg-gray-50 dark:bg-gray-800">
<div className="max-w-2xl mx-auto">
<h1 className="text-2xl font-bold mb-4 text-center">
<h1 className="text-2xl font-bold mb-4 text-center text-black dark:text-white">
Live GitHub Activity
</h1>

Expand Down
Loading