Skip to content

[ONCALL-187] logs: debug logs for no-proxy cx issue #245

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

Open
Aarushsr12 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[ONCALL-187] logs: debug logs for no-proxy cx issue #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fabac58
logs for no-proxy
Aarushsr12 Dec 11, 2025
8119cc9
more logs
Aarushsr12 Dec 11, 2025
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
102 changes: 55 additions & 47 deletions src/main/actions/startBackgroundProcess.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,61 +24,69 @@ const resolveBackgroundPath = (htmlFileName) => {

const startBackgroundProcess = async () => {
// eslint-disable-next-line no-async-promise-executor
console.log("DEBUG: startBackgroundProcess: Starting Background Process");
return new Promise(async (resolve) => {
let backgroundWindow = await getState("backgroundWindow");
if (!backgroundWindow) {
backgroundWindow = await setState(null, {
stateName: "backgroundWindow",
// Background Process Window
newValue: new BrowserWindow({
width: 800,
height: 600,
show:
process.env.NODE_ENV === "development" ||
process.env.DEBUG_PROD === "true",
webPreferences: {
nodeIntegration: true,
contextIsolation: false,
enableRemoteModule: true,
},
}),
});
enableWebContents(backgroundWindow.webContents);
} else {
logger.log(
"startBackgroundProcess: A background windows already exists. Cancelling."
);

resolve(true);
return;
}
try {
let backgroundWindow = await getState("backgroundWindow");
if (!backgroundWindow) {
backgroundWindow = await setState(null, {
stateName: "backgroundWindow",
// Background Process Window
newValue: new BrowserWindow({
width: 800,
height: 600,
show:
process.env.NODE_ENV === "development" ||
process.env.DEBUG_PROD === "true",
webPreferences: {
nodeIntegration: true,
contextIsolation: false,
enableRemoteModule: true,
},
}),
});
enableWebContents(backgroundWindow.webContents);
console.log("DEBUG: startBackgroundProcess: Background Window Created");
} else {
logger.log(
"startBackgroundProcess: A background windows already exists. Cancelling."
);

global.backgroundWindow = backgroundWindow;
resolve(true);
return;
}
Comment on lines +50 to +57
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, find the file and examine its size
wc -l src/main/actions/startBackgroundProcess.js

Repository: requestly/requestly-desktop-app

Length of output: 120

🏁 Script executed:

# Search for calls to startBackgroundProcess
rg -nP -C5 'startBackgroundProcess\s*\(' --type=js

Repository: requestly/requestly-desktop-app

Length of output: 57

🏁 Script executed:

# Read the entire startBackgroundProcess.js file
cat -n src/main/actions/startBackgroundProcess.js

Repository: requestly/requestly-desktop-app

Length of output: 3609

🏁 Script executed:

# Search for calls to startBackgroundProcess
rg -nP 'startBackgroundProcess' --type=js

Repository: requestly/requestly-desktop-app

Length of output: 937

🏁 Script executed:

# Search for uses of global.backgroundWindow
rg -nP 'global\.backgroundWindow' --type=js

Repository: requestly/requestly-desktop-app

Length of output: 648

🏁 Script executed:

# Search for uses of global.isBackgroundProcessActive
rg -nP 'global\.isBackgroundProcessActive' --type=js

Repository: requestly/requestly-desktop-app

Length of output: 167

🏁 Script executed:

# Check if there's a git history for this file to understand if this is a recent change
git log --oneline -10 src/main/actions/startBackgroundProcess.js

Repository: requestly/requestly-desktop-app

Length of output: 100

🏁 Script executed:

# Let's also check the exact state - is global.backgroundWindow defined elsewhere?
rg -nP 'global\.backgroundWindow\s*=' --type=js

Repository: requestly/requestly-desktop-app

Length of output: 170

Remove the early return or ensure global.backgroundWindow and initialization state are properly managed.

The early return when backgroundWindow already exists (lines 55-56) is problematic. If this function is called a second time when a window already exists:

  • global.backgroundWindow is never assigned (line 59 is skipped), leaving it undefined or stale
  • Code in initPrimaryStorage.js and cleanup.js that checks if (global.backgroundWindow) will fail silently
  • IPC forwarding is not re-established (line 75)
  • The did-finish-load event listener is not attached, so resolve(true) is called immediately rather than waiting for the window to finish loading

The log message "Cancelling" suggests this is a guard against multiple simultaneous calls, but the guard is incomplete. Either complete the full initialization sequence even when the window exists, or ensure that global.backgroundWindow and other global state are already set from the previous call before returning early.

🤖 Prompt for AI Agents 
In src/main/actions/startBackgroundProcess.js around lines 50-57, the early
return when a background window already exists skips setting or re-validating
global.backgroundWindow, skips re-establishing IPC forwarding and the
did-finish-load listener, and leaves initPrimaryStorage.js/cleanup.js checks
unreliable; remove the premature return and instead either (A) when a window
exists, reassign/validate global.backgroundWindow, reattach IPC forwarding
handlers, and if the window hasn’t finished loading attach the did-finish-load
listener so resolve(true) is only called after load, or (B) explicitly ensure
all global state (global.backgroundWindow and any initialization flags) were set
previously and still valid before returning; implement one of these flows so the
function always leaves global state and event/IPC wiring consistent and resolves
only after load completion.


// Load background code
backgroundWindow.loadURL(resolveBackgroundPath("index.html"));
global.backgroundWindow = backgroundWindow;

// Open the DevTools in dev mode
if (
process.env.NODE_ENV === "development" ||
process.env.DEBUG_PROD === "true"
)
{
backgroundWindow.webContents.once('dom-ready', () => {
backgroundWindow.webContents.openDevTools();
})
}
// Load background code
backgroundWindow.loadURL(resolveBackgroundPath("index.html"));

// Setup IPC forwarding
setupIPCForwardingToBackground(backgroundWindow);
// Open the DevTools in dev mode
if (
process.env.NODE_ENV === "development" ||
process.env.DEBUG_PROD === "true"
) {
backgroundWindow.webContents.once("dom-ready", () => {
backgroundWindow.webContents.openDevTools();
});
}

// Set state
global.isBackgroundProcessActive = true;
// Setup IPC forwarding
setupIPCForwardingToBackground(backgroundWindow);

backgroundWindow.webContents.on("did-finish-load", () => {
resolve(true);
});
// Set state
global.isBackgroundProcessActive = true;

backgroundWindow.webContents.on("did-finish-load", () => {
resolve(true);
});
} catch (error) {
console.log(
"DEBUG: startBackgroundProcess: Error starting background process",
error
);
resolve(false);
}
});
};

Expand Down
7 changes: 6 additions & 1 deletion src/renderer/actions/initEventHandlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,19 @@ import { getAvailableAndroidDevices } from "./apps/mobile/utils";
import { sendMessageToExtension } from "./helperSocketServer";
import IosSimulatorDevice from "./apps/mobile/iosSimulator";


const initEventHandlers = () => {
console.log("DEBUG:", "initializing IPC Event Handlers");
ipcRenderer.send("background-process-started");
ipcRenderer.on("start-proxy-server", async () => {
const PROXY_RESULT = await startProxyServer();
console.log("DEBUG:", "Proxy Server Started:", PROXY_RESULT);
ipcRenderer.send("reply-start-proxy-server", PROXY_RESULT);
ipcRenderer.send("proxy-config-updated", getProxyConfig());
});

ipcRenderer.on("detect-available-apps", async (event, payload) => {
console.log("DEBUG: detect-available-apps", payload);

const arrayOfAppIds = payload;
let final_result = {};

Expand All @@ -53,6 +56,8 @@ const initEventHandlers = () => {
.catch((e) => console.error("Unexpected Behaviour", e));
}

console.log("DEBUG: detect-available-apps final result", final_result);

ipcRenderer.send("reply-detect-available-apps", final_result);
});

Expand Down
38 changes: 33 additions & 5 deletions src/renderer/actions/proxy/startProxyServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ import startHelperServer, { stopHelperServer } from "../startHelperServer";
import logger from "utils/logger";
import { getDefaultProxyPort } from "../storage/cacheUtils";
import { handleCARegeneration } from "../apps/os/ca/utils";
import { startHelperSocketServer, stopHelperSocketServer } from "../helperSocketServer";
import {
startHelperSocketServer,
stopHelperSocketServer,
} from "../helperSocketServer";
import portfinder from "portfinder";
import LoggerService from "renderer/lib/proxy-interface/loggerService";
import { addShutdownHandler } from "../shutdown";
Expand Down Expand Up @@ -47,13 +50,24 @@ function startProxyFromModule(PROXY_PORT: number) {
RQProxyProvider.createInstance(
proxyConfig,
new RulesDataSource(),
new LoggerService(),
new LoggerService()
);
window.proxy = RQProxyProvider.getInstance().proxy;
console.log("DEBUG:", "Proxy assigned to window object");
}

export function stopProxyServer() {
if(window.proxy) {
// on startup window.proxy comes undefined
// but if refreshed or some earlier proxy was there it will show the proxy object
console.log(
"DEBUG:",
`onStartup window.proxy is undefined, else not: ${window.proxy}`
);
if (window.proxy) {
console.log(
"DEBUG:",
`onRefresh window.proxy will be there: ${window.proxy}`
);
window.proxy.close();
}
}
Expand All @@ -65,6 +79,7 @@ export default async function startProxyServer(
): Promise<IStartProxyResult> {
// Check if proxy is already listening. If so, close it
try {
console.log("DEBUG:", "stopProxyServerCalled");
stopProxyServer();
logger.log("A proxy server was closed");
} catch (error) {
Expand All @@ -74,6 +89,8 @@ export default async function startProxyServer(
const proxyIp = ip()!;
const targetPort = proxyPort || getDefaultProxyPort();

console.log(`DEBUG: ${proxyIp} ${targetPort}`);

const result: IStartProxyResult = {
success: true,
port: targetPort,
Expand All @@ -82,7 +99,9 @@ export default async function startProxyServer(

// start the proxy server
const FINAL_PROXY_PORT = await getNextAvailablePort(targetPort);
console.log("DEBUG: Is FINAL_PROXY_PORT is available", FINAL_PROXY_PORT);
if (!FINAL_PROXY_PORT) {
console.log("DEBUG: returning success as false");
result.success = false;
return result;
}
Expand All @@ -94,13 +113,18 @@ export default async function startProxyServer(

// start the helper server if not already running
if (shouldStartHelperServer) {
// No proxy comes for the case when there is success is false in result
console.log("DEBUG:", "starting up the helper server");
const HELPER_SERVER_PORT = await getNextAvailablePort(
DEFAULT_HELPER_SERVER_PORT
);

result.helperServerPort = HELPER_SERVER_PORT;

if (!HELPER_SERVER_PORT) {
console.log(
"DEBUG: returning success as false as no helper server port available"
);
result.success = false;
return result;
}
Expand All @@ -111,15 +135,19 @@ export default async function startProxyServer(
port: DEFAULT_SOCKET_SERVER_PORT,
stopPort: DEFAULT_SOCKET_SERVER_PORT + 4, // 5 ports for fallback
});
console.log(
"DEBUG:",
`${HELPER_SOCKET_SERVER_PORT} HELPER_SOCKET_SERVER_PORT is available`
);
startHelperSocketServer(HELPER_SOCKET_SERVER_PORT);

// fix-me: this does not remove existing handlers when restarting.
// fix-me: this does not remove existing handlers when restarting.
// For now that doesn't have side effects
addShutdownHandler(() => {
stopProxyServer();
stopHelperServer();
stopHelperSocketServer();
})
});
Comment on lines +144 to +150
Copy link

@coderabbitai coderabbitai bot Dec 11, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Potential memory leak from accumulating shutdown handlers.

The comment on line 142 correctly identifies that addShutdownHandler does not remove existing handlers when the proxy server is restarted. Each call to startProxyServer will add new handlers, potentially causing multiple identical handlers to execute on shutdown and accumulating memory over time.

Consider one of these approaches:

  1. Track and remove previous handlers before adding new ones
  2. Use a singleton pattern to ensure handlers are registered only once
  3. Clear all handlers before registration

Would you like me to generate a solution that prevents handler accumulation?

🤖 Prompt for AI Agents 
In src/renderer/actions/proxy/startProxyServer.ts around lines 142 to 148, add
logic to prevent accumulating shutdown handlers when startProxyServer is called
multiple times: either store the handler ID returned by addShutdownHandler and
remove the previous handler before adding a new one, or track a module-level
boolean/singleton to register the shutdown handlers only once; ensure
stopProxyServer/stopHelperServer/stopHelperSocketServer are still invoked on
shutdown and that any previous handler references are cleared or replaced to
avoid memory leaks.


return result;
}