Skip to content

fix: remove proxy for api client requests & resolve localhost requests #276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nsrCodes merged 2 commits into from
Feb 6, 2026
Merged

fix: remove proxy for api client requests & resolve localhost requests #276

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eaa1738
fix: remove proxy for api client & resolve localhost requests
Aarushsr12 Jan 29, 2026
f8dc0bf
fix
Aarushsr12 Jan 30, 2026
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
100 changes: 89 additions & 11 deletions src/main/actions/getProxiedAxios.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,54 @@ import axios, { AxiosInstance } from "axios";
import { readFileSync } from "fs";
import { HttpsProxyAgent, HttpsProxyAgentOptions } from "https-proxy-agent";
import { ClientRequest, RequestOptions } from "agent-base";
import { Socket } from "net";
import http from "http";
import https from "https";
import {
addCookiesToRequest,
storeCookiesFromResponse,
} from "./cookiesHelpers";

const LOCAL_IPV4 = "127.0.0.1";
const LOCAL_IPV6 = "::1";

const checkConnection = (host: string, port: number): Promise<boolean> => {
return new Promise((resolve) => {
const socket = new Socket();
const timeout = 1000;

socket.setTimeout(timeout);

socket.once("connect", () => {
socket.destroy();
resolve(true);
});

socket.once("timeout", () => {
socket.destroy();
resolve(false);
});

socket.once("error", () => {
socket.destroy();
resolve(false);
});

socket.connect(port, host);
});
};


const createLocalhostLookup = async (port: number) => {
const ipv6Works = await checkConnection(LOCAL_IPV6, port).catch(() => false);
const targetIp = ipv6Works ? LOCAL_IPV6 : LOCAL_IPV4;
const targetFamily = ipv6Works ? 6 : 4;

return (_lookupHostname: string, _options: any, callback: any) => {
callback(null, targetIp, targetFamily);
};
};

class PatchedHttpsProxyAgent extends HttpsProxyAgent {
ca: unknown;

Expand All @@ -32,17 +75,48 @@ let proxyConfig: ProxyConfig;

function createAxiosInstance(
config: ProxyConfig,
enableRQProxy: boolean = false,
addStoredCookies: boolean = false
): AxiosInstance {
const instance = axios.create({
proxy: false,
httpAgent: new HttpsProxyAgent(`http://${config.ip}:${config.port}`),
httpsAgent: new PatchedHttpsProxyAgent({
host: config.ip,
port: config.port,
ca: readFileSync(config.rootCertPath),
}),
});
let instance: AxiosInstance;
if (enableRQProxy) {
instance = axios.create({
proxy: false,
httpAgent: new HttpsProxyAgent(`http://${config.ip}:${config.port}`),
httpsAgent: new PatchedHttpsProxyAgent({
host: config.ip,
port: config.port,
ca: readFileSync(config.rootCertPath),
}),
});
} else {
instance = axios.create({
proxy: false,
});

instance.interceptors.request.use(async (requestConfig) => {
const { url: requestUrl } = requestConfig;

if (!requestUrl) {
return requestConfig;
}

const url = new URL(requestUrl);
const { hostname, port: urlPort, protocol } = url;

if (hostname === "localhost" || hostname === LOCAL_IPV6 || hostname === LOCAL_IPV4) {
// convert string port to integer
const port = urlPort ? parseInt(urlPort, 10) : protocol === "https:" ? 443 : 80;

const lookup = await createLocalhostLookup(port);

requestConfig.httpAgent = new http.Agent({ lookup });
requestConfig.httpsAgent = new https.Agent({ lookup });
Copy link
Collaborator

@nsrCodes nsrCodes Jan 29, 2026

Choose a reason for hiding this comment

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

why add it to https agent? is it needed there as well?

Copy link
Contributor Author

@Aarushsr12 Aarushsr12 Jan 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Not needed though, do local setups use https://localhost ?

Copy link
Collaborator

@nsrCodes nsrCodes Jan 30, 2026

Choose a reason for hiding this comment

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

you are right, no harm in keeping it

}
Comment on lines +97 to +115
Copy link

@coderabbitai coderabbitai bot Jan 30, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Read the file to see the context and current implementation
cat -n src/main/actions/getProxiedAxios.ts | head -130 | tail -50

Repository: requestly/requestly-desktop-app

Length of output: 1847

🏁 Script executed:

node <<'EOF'
// Test URL constructor with relative paths
console.log("Test 1: Absolute URL");
try {
  const url1 = new URL("https://example.com/path");
  console.log("✓ Success:", url1.href);
} catch (e) {
  console.log("✗ Error:", e.message);
}

console.log("\nTest 2: Relative URL without base");
try {
  const url2 = new URL("/path");
  console.log("✓ Success:", url2.href);
} catch (e) {
  console.log("✗ Error:", e.message);
}

console.log("\nTest 3: Relative URL with base parameter");
try {
  const url3 = new URL("/path", "https://example.com");
  console.log("✓ Success:", url3.href);
} catch (e) {
  console.log("✗ Error:", e.message);
}

console.log("\nTest 4: Simple relative path");
try {
  const url4 = new URL("api/endpoint");
  console.log("✓ Success:", url4.href);
} catch (e) {
  console.log("✗ Error:", e.message);
}

console.log("\nTest 5: Simple relative path with base");
try {
  const url5 = new URL("api/endpoint", "https://example.com");
  console.log("✓ Success:", url5.href);
} catch (e) {
  console.log("✗ Error:", e.message);
}
EOF

Repository: requestly/requestly-desktop-app

Length of output: 400

🏁 Script executed:

# Search for axios creation with baseURL
rg "axios\.create|baseURL" --type ts --type js -A 3 -B 1

Repository: requestly/requestly-desktop-app

Length of output: 703

🏁 Script executed:

# Search for usage of getProxiedAxios
rg "getProxiedAxios|createOrUpdateAxiosInstance" --type ts --type js -B 2 -A 5

Repository: requestly/requestly-desktop-app

Length of output: 3612

🏁 Script executed:

# Read makeApiClientRequest to see how axios is used
cat -n src/main/actions/makeApiClientRequest.js | head -120 | tail -80

Repository: requestly/requestly-desktop-app

Length of output: 2929

🏁 Script executed:

# Read the start of makeApiClientRequest to see the function signature and URL source
cat -n src/main/actions/makeApiClientRequest.js | head -40

Repository: requestly/requestly-desktop-app

Length of output: 1599

Guard URL parsing against relative url values.

new URL(requestUrl) throws for relative paths, which will crash the interceptor and prevent requests from being sent. This is particularly important for applications using axios with baseURL or for handling edge cases with relative URLs. Extract baseURL from requestConfig and use it as a fallback parameter to the URL constructor, returning early if parsing fails.

🛠️ Proposed fix 
-    instance.interceptors.request.use(async (requestConfig) => {
-      const { url: requestUrl } = requestConfig;
+    instance.interceptors.request.use(async (requestConfig) => {
+      const { url: requestUrl, baseURL } = requestConfig;
 
       if (!requestUrl) {
         return requestConfig;
       }
 
-      const url = new URL(requestUrl);
+      let url: URL;
+      try {
+        url = baseURL ? new URL(requestUrl, baseURL) : new URL(requestUrl);
+      } catch {
+        return requestConfig;
+      }
       const { hostname, port: urlPort, protocol } = url;
📝 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
instance.interceptors.request.use(async (requestConfig) => {
const { url: requestUrl } = requestConfig;
if (!requestUrl) {
return requestConfig;
}
const url = new URL(requestUrl);
const { hostname, port: urlPort, protocol } = url;
if (hostname === "localhost" || hostname === LOCAL_IPV6 || hostname === LOCAL_IPV4) {
// convert string port to integer
const port = urlPort ? parseInt(urlPort, 10) : protocol === "https:" ? 443 : 80;
const lookup = await createLocalhostLookup(port);
requestConfig.httpAgent = new http.Agent({ lookup });
requestConfig.httpsAgent = new https.Agent({ lookup });
}
instance.interceptors.request.use(async (requestConfig) => {
const { url: requestUrl, baseURL } = requestConfig;
if (!requestUrl) {
return requestConfig;
}
let url: URL;
try {
url = baseURL ? new URL(requestUrl, baseURL) : new URL(requestUrl);
} catch {
return requestConfig;
}
const { hostname, port: urlPort, protocol } = url;
if (hostname === "localhost" || hostname === LOCAL_IPV6 || hostname === LOCAL_IPV4) {
// convert string port to integer
const port = urlPort ? parseInt(urlPort, 10) : protocol === "https:" ? 443 : 80;
const lookup = await createLocalhostLookup(port);
requestConfig.httpAgent = new http.Agent({ lookup });
requestConfig.httpsAgent = new https.Agent({ lookup });
}
🤖 Prompt for AI Agents 
In `@src/main/actions/getProxiedAxios.ts` around lines 97 - 115, The request
interceptor crashes on relative URLs because new URL(requestUrl) throws; update
instance.interceptors.request.use to guard URL parsing by using
requestConfig.baseURL as the fallback (i.e., try new URL(requestUrl,
requestConfig.baseURL)) and wrap construction in a try/catch that returns
requestConfig on parse failure; only proceed to check hostname/port and call
createLocalhostLookup when URL construction succeeds, and keep setting
requestConfig.httpAgent/httpsAgent as before.

Copy link
Contributor Author

@Aarushsr12 Aarushsr12 Jan 30, 2026

Choose a reason for hiding this comment

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

can you explain this by some examples ?

Copy link

@coderabbitai coderabbitai bot Jan 30, 2026

Choose a reason for hiding this comment

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

@Aarushsr12, sure! Let me explain with concrete examples:

The Problem

The new URL() constructor requires an absolute URL (with protocol). If you pass a relative URL, it throws an error:

// ✅ Works - absolute URL
new URL('https://api.example.com/users')
// Result: { hostname: 'api.example.com', ... }

// ❌ Crashes - relative URL
new URL('/api/users')
// Throws: TypeError: Invalid URL

// ❌ Crashes - relative path
new URL('api/users')
// Throws: TypeError: Invalid URL

Why This Matters for Axios

When using axios, you can configure requests in different ways:

// Scenario 1: Absolute URL - works fine
axios.get('https://api.example.com/users')

// Scenario 2: Using baseURL with relative path - CRASHES with current code
const client = axios.create({ baseURL: 'https://api.example.com' })
client.get('/users')  // requestUrl = '/users' → new URL('/users') throws!

// Scenario 3: Just a path - CRASHES with current code
axios.get('/api/users')

The Fix

By using baseURL as the second parameter to the URL constructor, we can safely parse relative URLs:

// Before (crashes):
const url = new URL('/users')  // ❌ Throws

// After (works):
const url = new URL('/users', 'https://api.example.com')  // ✅ Returns full URL

// The try-catch ensures any parsing errors don't break the request:
try {
  url = baseURL ? new URL(requestUrl, baseURL) : new URL(requestUrl);
} catch {
  return requestConfig;  // Skip localhost handling, let axios handle it normally
}

Practical Impact

Without the fix, if your API client code uses:

const apiClient = getProxiedAxios();
apiClient.defaults.baseURL = 'https://api.requestly.io';
apiClient.get('/users');  // This would crash the interceptor

With the fix, the interceptor gracefully handles it by constructing the full URL and checking if it's localhost before applying the custom DNS lookup.

Does this clarify the issue?


return requestConfig;
});
}

instance.interceptors.response.use(storeCookiesFromResponse);
if (addStoredCookies) {
Expand All @@ -60,8 +134,12 @@ export const createOrUpdateAxiosInstance = (
};

try {
proxiedAxios = createAxiosInstance(proxyConfig);
proxiedAxiosWithSessionCookies = createAxiosInstance(proxyConfig, true);
proxiedAxios = createAxiosInstance(proxyConfig, false);
proxiedAxiosWithSessionCookies = createAxiosInstance(
proxyConfig,
false,
true
);
} catch (error) {
/* Do nothing */
console.error("Error creating or updating Axios instance:", error);
Expand Down