Skip to content

fix: isolate callback failures from HTTP workers - #69

Merged
hjianbo merged 3 commits into
mainfrom
fix/callback-worker-isolation
Sep 3, 2026
Merged

fix: isolate callback failures from HTTP workers#69
hjianbo merged 3 commits into
mainfrom
fix/callback-worker-isolation

Conversation

@hjianbo

@hjianbo hjianbo commented Sep 2, 2026

Copy link
Copy Markdown
Member

Summary

This is a robustness fix for the ehttpc worker pool.

Async callbacks are provided by callers, so an exception in callback code must not be able to take down the HTTP transport worker. Before this change, callbacks were evaluated directly in the ehttpc worker process. An unexpected upstream response could therefore kill a worker, and repeated failures could exhaust the supervisor restart intensity and leave the whole pool unavailable until the resource was restarted.

The fix keeps callback failures contained while preserving the existing request representations on both synchronous and asynchronous hot paths. Invalid callback shapes are rejected before dispatch, and malformed callback values cannot be passed to gen_server:reply/2. Callback failures are logged without including the HTTP response payload.

What changed

  • Validate the callback shape before sending an async request.
  • Distinguish callbacks from synchronous replies with guarded dispatch, without adding target wrappers to request state.
  • Catch callback failures from response, transport error, timeout, and stream-ref paths.
  • Preserve the existing callback forms, argument ordering, and execution timing for valid callers.
  • Add regression coverage for callback failures, malformed callbacks, timeout callbacks, multi-argument callbacks, and repeated failures.

Reference incident

The business-level response parsing problem is tracked separately in emqx/emqx_whc#142. This PR is the ehttpc-side robustness fix.

The following redacted excerpts from that incident show why callback failures must not be allowed to terminate the shared pool:

2026-08-29T10:39:07.537453+08:00 [error]
Supervisor: {<...>,ehttpc_worker_sup}.
Context: child_terminated.
Reason: {{case_clause,{ok,#{<<"code">> => 0,<<"data">> => [...]}}}, ...}.
Offender: id={whc_http_dm_pool,...}

2026-08-29T10:39:12.230851+08:00 [error]
Supervisor: {<...>,ehttpc_worker_sup}.
Context: shutdown.
Reason: reached_max_restart_intensity.
Offender: id={whc_http_dm_pool,...}

2026-08-29T10:39:27.555045+08:00 [error]
stacktrace: [{ehttpc,request_async,[false,...]}, ...].
hint: <<"Unknown error: function_clause">>

@hjianbo
hjianbo marked this pull request as ready for review September 2, 2026 03:40
Comment thread src/ehttpc.erl Outdated
@hjianbo
hjianbo requested a review from zmstone September 2, 2026 08:07
Comment thread src/ehttpc.erl Outdated
@hjianbo
hjianbo merged commit 95198aa into main Sep 3, 2026
4 checks passed
@hjianbo
hjianbo deleted the fix/callback-worker-isolation branch September 3, 2026 07:44
@hjianbo hjianbo self-assigned this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants