Skip to content

fix: prevent process crash from sync throw in forward() - #737

Open
VojtaM39 wants to merge 1 commit into
masterfrom
fix/forward-process-crash
Open

VojtaM39 wants to merge 1 commit into
masterfrom
fix/forward-process-crash

Conversation

@VojtaM39

Copy link
Copy Markdown
Contributor

What

forward() and forwardSocks() used new Promise(async (resolve, reject) => ...). A synchronous throw inside
the executor (e.g. http.request throwing ERR_INVALID_IP_ADDRESS when prepareRequestFunction returns an
invalid localAddress) became an unhandled rejection, which crashes the process on Node >= 15, and the outer
promise never settled so the client request hung.

How

Executors are now synchronous. A throw rejects the promise, Server.onRequest catches it, the client gets a 500
and requestFailed is emitted.

Tests

Added e2e test returns 500 when forward() throws synchronously. It fails on master (unhandled rejection +
hang).

@apify-service-account apify-service-account added the tested Temporary label used only programatically for some analytics. label Oct 29, 2026
@apify-service-account apify-service-account added the t-unblocking Issues with this label are in the ownership of the unblocking team. label Oct 29, 2026
@VojtaM39 VojtaM39 added adhoc Ad-hoc unplanned task added during the sprint. security Security related issue. labels Oct 29, 2026
@VojtaM39
VojtaM39 requested a review from bliuchak October 29, 2026 15:22
@bliuchak

Copy link
Copy Markdown
Contributor

Good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. security Security related issue. t-unblocking Issues with this label are in the ownership of the unblocking team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants