fix: localhost resolution error

[nsrCodes]

• nodejs 17+ changed dns resolution to match the results returned from the os Ref
• the httpProxyAgent (that we recently removed - [ONCALL-125] chore: stop api client request to be proxied #214) in the axios config was earlier managing the fallback to this and resolving through ipv4 for loopback addresses Ref

Two ways to solve this could be:

• change the dns result order from verbatim to ipv4first to force ipv4 resolution for all cases. but this would impact all requests and not just loopback address. hence, this might have unexpected consequences
• make a custom agent that handles this specifically

This PR proposes changes for the second approach

Summary by CodeRabbit

• Bug Fixes
  • Fixed localhost connection resolution to ensure consistent IPv4 protocol usage, improving network reliability across different proxy configurations.

[coderabbitai]

Walkthrough

This pull request modifies the Axios proxy configuration to add IPv4-specific DNS resolution for localhost in the non-Requestly proxy path. The changes include importing the http and https modules, implementing a localhostIPv4Lookup function to force IPv4 resolution, creating custom httpAgent and httpsAgent with this lookup function, and passing these agents to the Axios configuration. The Requestly proxy path retains its existing behavior unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Single-file modification with focused scope
• Custom DNS lookup function requires verification for correctness and edge case handling
• Potential concern: DNS lookup behavior changes could have subtle networking implications
• Consider verifying IPv4 vs IPv6 resolution logic and ensuring no regression in the Requestly proxy path

Possibly related PRs

• [ONCALL-125] chore: stop api client request to be proxied #214: Modifies the same src/main/actions/getProxiedAxios.ts file to control proxy agent configuration; related to overall proxy handling improvements in the Axios setup.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: fixing localhost resolution errors by configuring DNS to prioritize IPv4, which aligns with the PR's core objective and file modifications.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-localhost-resolution-error

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/main/actions/getProxiedAxios.ts (2)

9-11: Use consistent import syntax.

Line 9 uses CommonJS require() while lines 10-11 use ES6 import syntax. Consider using ES6 imports consistently for better TypeScript integration and code clarity.

Apply this diff for consistency:

-const dns = require("dns")
+import dns from "dns";
 import http from "http";
 import https from "https";

---

54-60: Add proper type annotations to the lookup function.

The function parameters are typed as any, which reduces type safety. Consider using proper types from Node.js dns module.

Apply this diff to add type annotations:

-    const localhostIPv4Lookup = (hostname: any, options: any, callback: any) => {
+    const localhostIPv4Lookup = (
+      hostname: string,
+      options: dns.LookupOneOptions,
+      callback: (err: NodeJS.ErrnoException | null, address: string, family: number) => void
+    ) => {
       if (hostname === 'localhost') {
         dns.lookup(hostname, { ...options, family: 4 }, callback);
       } else {
         dns.lookup(hostname, options, callback);
       }
     };

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c6cacd5 and 91dbfba.

📒 Files selected for processing (1)

• src/main/actions/getProxiedAxios.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

src/main/actions/getProxiedAxios.ts (1)

src/main/actions/makeApiClientRequest.js (1)

• axios (70-70)

🔇 Additional comments (3)

src/main/actions/getProxiedAxios.ts (3)

61-62: LGTM! Custom agents properly configured.

The http and https agents are correctly instantiated with the custom localhostIPv4Lookup function, ensuring IPv4-first resolution for localhost addresses.

---

63-67: LGTM! Axios instance correctly configured with custom agents.

The custom agents are properly integrated into the Axios instance configuration for the non-RQ-proxy path, implementing the IPv4-first localhost resolution as intended.

---

54-60: No issues identified. The limited scope is intentional and correct.

The localhostIPv4Lookup function is specifically designed to handle the 'localhost' hostname by forcing IPv4-only resolution. This is necessary because Node.js resolves 'localhost' to both IPv4 (127.0.0.1) and IPv6 (::1) by default, and the proxy is only available on IPv4.

Numeric IPv4 addresses like 127.0.0.1 don't require special DNS lookup handling—they're already resolved. IPv6 loopback addresses (::1) are not used anywhere in the codebase, and no scenarios require their special handling.

The design appropriately addresses the actual localhost resolution scenario in use.