-
-
Notifications
You must be signed in to change notification settings - Fork 26.5k
Description
Describe the bug
Bug Description
The application crashes with TypeError: Cannot read property 'data' of undefined
when network errors occur
(timeouts, DNS failures, connection refused, etc.) due to
unsafe property access on err.response
in the error
handling code.
Affected Files
src/common/retryer.js:66-69
(Critical - affects all
GitHub API calls)src/fetchers/wakatime.js:24
(Critical - affects
Wakatime stats fetching)
Root Cause
When axios encounters network-level errors (ECONNREFUSED,
ETIMEDOUT, ENOTFOUND, etc.), the error object does not
contain a response
property. The current code attempts to
access err.response.data
and err.response.status
directly without null-safety checks, causing crashes.
Problematic Code
src/common/retryer.js:66-69:
} catch (err) {
// ❌ CRASHES if err.response is undefined
const isBadCredential = err.response.data &&
err.response.data.message === "Bad credentials";
const isAccountSuspended =
err.response.data &&
err.response.data.message === "Sorry. Your account was
suspended.";
if (isBadCredential || isAccountSuspended) {
logger.log(`PAT_${retries + 1} Failed`);
retries++;
return retryer(fetcher, variables, retries);
} else {
return err.response; // Returns undefined if
err.response doesn't exist
}
}
src/fetchers/wakatime.js:24:
} catch (err) {
// ❌ CRASHES if err.response is undefined
if (err.response.status < 200 || err.response.status >
299) {
throw new CustomError(
`Could not resolve to a User with the login of
'${username}'`,
"WAKATIME_USER_NOT_FOUND",
);
}
throw err;
}
Impact
- Severity: 🔴 HIGH
- User Impact: Complete API endpoint failure with 500 errors
- Frequency: Occurs during network instability, DNS issues,
or GitHub API unavailability - Scope: Affects all users attempting to generate stats
cards during network issues
Steps to Reproduce
- Simulate network timeout or connection failure to GitHub
API - Attempt to fetch GitHub stats
- Observe TypeError: Cannot read property 'data' of
undefined crash - API returns 500 error instead of graceful error handling
Quick reproduction (for testing):
// Simulate network error without response object
const networkError = new Error('ECONNREFUSED');
// networkError.response is undefined
Expected Behavior
The application should gracefully handle network errors and
either:
- Retry with the next available token
- Return a user-friendly error message
- Not crash the entire API endpoint
Actual Behavior
Application crashes with unhandled exception, breaking the
API endpoint completely.
Proposed Solution
Use optional chaining (?.) for safe property access:
Fix for src/common/retryer.js:
} catch (err) {
// ✅ Safe property access
const isBadCredential = err.response?.data?.message ===
"Bad credentials";
const isAccountSuspended = err.response?.data?.message ===
"Sorry. Your account was suspended.";
if (isBadCredential || isAccountSuspended) {
logger.log(`PAT_${retries + 1} Failed`);
retries++;
return retryer(fetcher, variables, retries);
} else {
// Handle case where err.response doesn't exist
return err.response || { data: { errors: [{ message:
err.message }] } };
}
}
Fix for src/fetchers/wakatime.js:
} catch (err) {
// ✅ Safe property access with fallback
const status = err.response?.status;
if (status && (status < 200 || status > 299)) {
throw new CustomError(
`Could not resolve to a User with the login of
'${username}'`,
"WAKATIME_USER_NOT_FOUND",
);
}
throw err;
}
Evidence of Inconsistency
Interestingly, api/status/pat-info.js:97 already uses the
correct pattern:
const errorMessage =
err.response?.data?.message?.toLowerCase(); //
✅ Correct
This indicates awareness of the issue in newer code, but
older error handlers weren't updated.
Additional Recommendations
- Add test coverage for network error scenarios in
tests/retryer.test.js - Audit all err.response usage across the codebase for
consistency - Add ESLint rule to prevent unsafe optional property
access on error objects
Related Code
- Git blame shows lines 66-69 in retryer.js date back to
2020-2023 - Recent commit ba9406e added comments about rate limit
detection but didn't fix the unsafe access
Environment
- Node.js: v22+ (as per package.json engines)
- All deployment environments affected (Vercel, self-hosted)
Expected behavior
No response
Screenshots / Live demo link
No response
Additional context
No response