test(logger): add e2e test for ENOTFOUND error log#1233
Conversation
📝 WalkthroughWalkthroughThis PR narrows a cast in the logger plugin so ChangesProxy Error Logger
🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/plugins/default/logger-plugin.ts`:
- Line 29: The log builds targetHref unsafely by casting target to URL, causing
the string "undefined" when target is absent; change the derivation to safely
check the payload: if target is an instance of URL use target.href, else if
target is an object with a href property coerce that to string, otherwise use an
empty string (or "unknown") — update the expression where targetHref is defined
(the const targetHref in logger-plugin.ts) to perform these runtime checks
instead of an unchecked cast.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2df43af5-0677-4198-906f-a9c5db46e73d
📒 Files selected for processing (2)
src/plugins/default/logger-plugin.tstest/e2e/logger.spec.ts
39cb266 to
abacb50
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/plugins/default/logger-plugin.ts (1)
29-29:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winTest failures confirm
targetis undefined during ENOTFOUND errors.The pipeline failures show the test receives
"undefined"instead of"http://does-not-exist.invalid"in the logged error message. This confirms the existing review comment: whentargetis undefined, the cast toURLand optional chaining produceundefined, which stringifies to"undefined"in the template literal. The fix should fall back tooptions.targetwhen thetargetparameter is absent.🤖 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/plugins/default/logger-plugin.ts` at line 29, The logged targetHref is becoming the string "undefined" because (target as unknown as URL)?.href yields undefined when target is missing; change construction of targetHref to use options.target as a fallback (e.g., derive a string from target?.href || options.target) so that when target is undefined the log uses options.target instead; update the code that defines targetHref (referencing the target variable and options.target) to coalesce to a sensible string rather than allowing "undefined".
🤖 Prompt for all review comments with 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.
Duplicate comments:
In `@src/plugins/default/logger-plugin.ts`:
- Line 29: The logged targetHref is becoming the string "undefined" because
(target as unknown as URL)?.href yields undefined when target is missing; change
construction of targetHref to use options.target as a fallback (e.g., derive a
string from target?.href || options.target) so that when target is undefined the
log uses options.target instead; update the code that defines targetHref
(referencing the target variable and options.target) to coalesce to a sensible
string rather than allowing "undefined".
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d0844273-3afa-4756-ae64-7ae490c8dfad
📒 Files selected for processing (2)
src/plugins/default/logger-plugin.tstest/e2e/logger.spec.ts
add missing e2e logger test for ENOTFOUND
related: unjs/httpxy#135
Summary by CodeRabbit
Refactor
Tests