feat: Timeout handling and additional plugin pool improvements

[zeljkoX]

Summary

When a plugin exceeds its timeout, the API previously returned a generic HTTP 500 with "Internal server error". This PR makes timeouts actionable:

• Structured 504 response — ScriptTimeout now routes through the Handler path instead of Fatal, returning HTTP 504 with {code: "TIMEOUT"} and a descriptive
  message
• Timeout hierarchy — Adds a 2s buffer to the Rust-side timeout so Node.js responds first with a structured error; Rust acts as a backstop
• EPIPE resilience — Pool server no longer crashes (process.exit(1)) when a plugin times out mid-RPC call; EPIPE/ECONNRESET are expected during timeout
  teardown and handled gracefully
• Dead-server detection hardening — is_dead_server_error now matches on enum variants (ScriptTimeout, HandlerError) instead of fragile string parsing,
  preventing false-positive server restarts
• Socket write error levels — Broken pipe errors in shared socket writes downgraded from WARN to DEBUG (expected during timeouts)
• Config cleanup — Removed PLUGIN_POOL_REQUEST_TIMEOUT_SECS in favor of per-plugin timeouts from config.json

Testing Process

Checklist

• Add a reference to related issues in the PR description.
• Add unit tests if applicable.

Note

If you are using Relayer in your stack, consider adding your team or organization to our list of Relayer Users in the Wild!

Summary by CodeRabbit

• New Features

  • Added 504 status codes for plugin execution timeouts with clear timeout messages.
  • Introduced graceful error handling for client disconnections to reduce spurious error logs.

• Documentation

  • Updated timeout configuration guidance with dual-timeout model (per-plugin + HTTP request timeouts).
  • Added warnings to prevent write errors by aligning HTTP timeout with plugin timeout.
  • Enhanced troubleshooting section with timeout error solutions.

• Bug Fixes

  • Improved socket error handling during client disconnections.
  • Refined timeout detection in error classification.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 42a196dd-0bce-4084-8c85-ea5684bf432a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

Walkthrough

This PR restructures the plugin timeout architecture from a single pool-request timeout to a dual-tier model: per-plugin execution timeouts (via config.json, default 300s) and HTTP server request timeout, with corresponding updates to constants, error handling, and Rust-side backstop logic across both Node.js and Rust implementations.

Changes

Cohort / File(s)	Summary
Documentation & Architecture docs/plugins/index.mdx, plugins/ARCHITECTURE.md	Updated plugin configuration guidance to replace the old fixed pool timeout model with per-plugin timeouts (config.json) and HTTP REQUEST_TIMEOUT_SECONDS. Added warnings about alignment, EPIPE error troubleshooting, and removed references to PLUGIN_POOL_REQUEST_TIMEOUT_SECS.
TypeScript Timeout Constants & Refactoring plugins/lib/constants.ts, plugins/lib/compiler.ts, plugins/lib/plugin.ts, plugins/lib/pool-executor.ts, plugins/lib/worker-pool.ts	Replaced DEFAULT_POOL_EXECUTION_TIMEOUT_MS and DEFAULT_SOCKET_REQUEST_TIMEOUT_MS with SOCKET_REQUEST_TIMEOUT_MS (30s) and DEFAULT_PLUGIN_TIMEOUT_MS (300s). Updated all imports and usages to reference new constants; removed intermediate DEFAULT_TIMEOUT alias in worker-pool.ts.
Node.js Socket Error Handling plugins/lib/pool-server.ts	Introduced safeWrite wrapper to gracefully handle EPIPE and ECONNRESET errors during socket writes. Enhanced uncaughtException/unhandledRejection handlers to ignore expected socket errors during disconnect instead of terminating, with added logging and early-exit checks for socket.writable.
Rust Plugin Constants src/constants/plugins.rs	Removed DEFAULT_POOL_EXECUTION_TIMEOUT_MS and DEFAULT_POOL_REQUEST_TIMEOUT_SECS. Added PLUGIN_TIMEOUT_BUFFER_SECONDS (2s) for Rust-side backstop and ADMIN_REQUEST_TIMEOUT_SECS (30s) for admin operations (precompile, cache, invalidate).
Rust Configuration Structure src/services/plugins/config.rs	Removed pool_request_timeout_secs field from PluginConfig struct and eliminated parsing of PLUGIN_POOL_REQUEST_TIMEOUT_SECS environment variable and related constructor/Default initialization.
Rust Timeout & Error Handling src/services/plugins/connection.rs, src/services/plugins/mod.rs, src/services/plugins/pool_executor.rs	Updated timeout error mapping to ScriptTimeout variant; added ScriptTimeout handler returning 504 with timeout-specific message. Replaced pool timeout configuration with computed timeout (DEFAULT_PLUGIN_TIMEOUT_SECONDS + PLUGIN_TIMEOUT_BUFFER_SECONDS) and ADMIN_REQUEST_TIMEOUT_SECS. Refined dead-server detection to exclude ScriptTimeout and timeout-related error strings.
Rust Socket Logging src/services/plugins/shared_socket.rs	Introduced log_socket_write_error helper to centralize socket write error logging, treating BrokenPipe/ConnectionReset as DEBUG (expected) and other IO errors as WARN (unexpected).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• feat: Socket simplification #684: Directly modifies plugin socket/pool code, constants, and pool executor/shared socket handling with related timeout and socket management changes.
• feat: Plugin performance improvements #606: Modifies the same plugin subsystem files (constants, plugin.ts, pool-server.ts, worker-pool.ts) with overlapping timeout and socket/pool request handling logic.
• feat: Improve ready route, expose more stats #632: Touches plugin pool and shared-socket implementations; PR modifies timeout/backstop and socket error handling while related PR adds health/statistics collection on the same files.

Suggested labels

cla: allowlist

Suggested reviewers

• tirumerla
• NicoMolinaOZ
• collins-w

Poem

🐰 A timeout tale, once tangled and tight,
Now splits into layers—per-plugin, just right!
With buffer and admin, and sockets so wise,
No more EPIPE tears—just clear, working skies! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures the main changes: improved timeout handling (structured 504 responses, Rust-side buffer, EPIPE resilience) and plugin pool improvements (dead-server detection hardening, config cleanup).
Description check	✅ Passed	The description comprehensively covers the changes with clear sections for Summary and Checklist, though the Testing Process section is empty and checklist items remain unchecked.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch plugin-pool-improvements

📝 Coding Plan

• Generate coding plan for human review comments

---

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

Tip

CodeRabbit can use OpenGrep to find security vulnerabilities and bugs across 17+ programming languages.

OpenGrep is compatible with Semgrep configurations. Add an opengrep.yml or semgrep.yml configuration file to your project to enable OpenGrep analysis.

[codecov]

Codecov Report

❌ Patch coverage is 87.76978% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.97%. Comparing base (46e29bc) to head (aa88664).

Files with missing lines	Patch %	Lines
src/services/plugins/pool_executor.rs	75.00%	9 Missing ⚠️
src/services/plugins/shared_socket.rs	84.84%	5 Missing ⚠️
src/services/plugins/connection.rs	33.33%	2 Missing ⚠️
src/services/plugins/mod.rs	98.36%	1 Missing ⚠️

Additional details and impacted files

Flag	Coverage Δ
ai	0.26% <0.00%> (-0.01%)	⬇️
dev	90.96% <87.76%> (+<0.01%)	⬆️
properties	0.01% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@           Coverage Diff            @@
##             main     #691    +/-   ##
========================================
  Coverage   90.97%   90.97%            
========================================
  Files         288      288            
  Lines      118596   118698   +102     
========================================
+ Hits       107888   107984    +96     
- Misses      10708    10714     +6     

Files with missing lines	Coverage Δ
src/domain/transaction/stellar/prepare/fee_bump.rs	94.65% <100.00%> (+0.01%)	⬆️
src/services/plugins/config.rs	94.29% <ø> (-0.08%)	⬇️
src/services/plugins/mod.rs	98.14% <98.36%> (-0.30%)	⬇️
src/services/plugins/connection.rs	93.79% <33.33%> (-0.44%)	⬇️
src/services/plugins/shared_socket.rs	95.88% <84.84%> (+0.01%)	⬆️
src/services/plugins/pool_executor.rs	77.20% <75.00%> (+0.09%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[coderabbitai]

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/plugins/index.mdx`:
- Around line 847-853: Update the docs to recommend a buffer so
REQUEST_TIMEOUT_SECONDS is strictly greater than your longest plugin timeout
(plugin.timeout) to avoid the race that causes write EPIPE; explicitly state
"REQUEST_TIMEOUT_SECONDS should be at least X seconds greater than the longest
plugin.timeout (e.g., +1–2s buffer)" and apply the same change to the other
occurrence referenced (around the lines noted).

In `@plugins/lib/worker-pool.ts`:
- Around line 107-108: The outer worker-pool uses a fixed DEFAULT_TASK_TIMEOUT
(DEFAULT_PLUGIN_TIMEOUT_MS + 5000) which ignores per-plugin request.timeout,
causing plugins with timeouts > DEFAULT_PLUGIN_TIMEOUT_MS to be cut off; update
the code that sets taskTimeout (the value passed into the Piscina safety timeout
/ Promise.race) to derive from the effective per-request timeout (use
request.timeout when present, otherwise DEFAULT_PLUGIN_TIMEOUT_MS), then add the
5000ms cleanup buffer and use that computed value instead of
DEFAULT_TASK_TIMEOUT when invoking Piscina.runTask / Promise.race so the pool
honors per-plugin timeouts.

In `@src/services/plugins/pool_executor.rs`:
- Around line 996-1000: The response_timeout calculation (using
response_timeout, timeout_secs, DEFAULT_PLUGIN_TIMEOUT_SECONDS) adds a fixed 5s
buffer but still can be shorter than the actual worker-side wait, causing
callers to get a generic timeout before the worker can return the structured
504/TIMEOUT; update the response_timeout computation to match or exceed the
worker wait by using the same buffer/delay logic the worker uses (replace the
hardcoded + Duration::from_secs(5) with the same buffer constant or value used
by the worker, or compute response_timeout =
(timeout_secs.unwrap_or(DEFAULT_PLUGIN_TIMEOUT_SECONDS) + WORKER_BUFFER_SECS +
QUEUE_BUFFER_SECS) seconds) and apply the same change to the other occurrence
referenced in the comment (the block around the second occurrence at the later
lines) so both code paths use the identical timeout math.
- Around line 1096-1118: The extra broad string check in is_dead_server_error is
preventing DeadServerIndicator::from_error_str from detecting transport-level
timeouts (e.g., PluginExecutionError("connection timed out")); remove the
(lower.contains("plugin") && lower.contains("timed out")) clause and only
special-case the explicit Node.js phrase "handler timed out" so that
connection/transport timeout messages fall through to
DeadServerIndicator::from_error_str for proper dead-server detection.
- Around line 545-549: The current code adds PLUGIN_TIMEOUT_BUFFER_SECONDS into
the timeout passed to conn.send_request_with_timeout, causing ScriptTimeout to
report the expanded wall-clock time; instead pass the original configured
timeout (timeout_secs.unwrap_or(DEFAULT_PLUGIN_TIMEOUT_SECONDS)) to
send_request_with_timeout so callers see the intended timeout, and implement the
backstop separately (e.g., run a parallel watchdog using
PLUGIN_TIMEOUT_BUFFER_SECONDS that forcibly terminates the Node process if the
plugin hangs) so the buffer is used only for enforcement and not reflected in
the reported PluginError; update the call site where timeout is computed and the
call conn.send_request_with_timeout(&request, timeout) to use the
configured_timeout and add a watchdog/select-based backstop surrounding that
call.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: aecb0fd2-2cee-4211-a843-896e8b3b89a7

📥 Commits

Reviewing files that changed from the base of the PR and between de7890b and e3aa647.

📒 Files selected for processing (14)

• docs/plugins/index.mdx
• plugins/ARCHITECTURE.md
• plugins/lib/compiler.ts
• plugins/lib/constants.ts
• plugins/lib/plugin.ts
• plugins/lib/pool-executor.ts
• plugins/lib/pool-server.ts
• plugins/lib/worker-pool.ts
• src/constants/plugins.rs
• src/services/plugins/config.rs
• src/services/plugins/connection.rs
• src/services/plugins/mod.rs
• src/services/plugins/pool_executor.rs
• src/services/plugins/shared_socket.rs

💤 Files with no reviewable changes (1)

• plugins/ARCHITECTURE.md

[Copilot]

Pull request overview

This PR improves plugin pool timeout behavior and resiliency so that plugin execution timeouts return actionable, structured responses (HTTP 504 / TIMEOUT) and expected socket errors during timeout teardown don’t crash the pool server or spam logs.

Changes:

• Route ScriptTimeout through the normal handler path to return structured HTTP 504 responses (with code: "TIMEOUT").
• Implement a layered timeout hierarchy (handler → worker-pool safety net → Rust backstop) with explicit buffer constants.
• Harden pool-server / socket write behavior during disconnects (EPIPE/ECONNRESET) and simplify timeout-related configuration.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/services/plugins/shared_socket.rs	Downgrades expected socket write errors to DEBUG via a helper and adds basic tests.
src/services/plugins/pool_executor.rs	Adds timeout buffering/backstop logic, replaces request-timeout config usage with constants, and refines dead-server detection.
src/services/plugins/mod.rs	Converts ScriptTimeout into a structured handler response (504, TIMEOUT) and adds test coverage.
src/services/plugins/connection.rs	Changes pool request timeout mapping to PluginError::ScriptTimeout.
src/services/plugins/config.rs	Removes pool_request_timeout_secs from config.
src/constants/plugins.rs	Introduces timeout-buffer/admin-timeout constants and removes old pool request timeout constants.
plugins/lib/worker-pool.ts	Uses per-request timeout to derive worker safety-net timeout (avoids cutting off long-timeout plugins).
plugins/lib/pool-server.ts	Adds safeWrite and ignores expected disconnect errors in global error handlers.
plugins/lib/pool-executor.ts	Renames socket request timeout constant usage.
plugins/lib/plugin.ts	Renames socket request timeout constant usage.
plugins/lib/constants.ts	Splits execution vs per-API-call socket timeouts; introduces DEFAULT_PLUGIN_TIMEOUT_MS.
plugins/lib/compiler.ts	Minor comment tweak for compile timeout.
plugins/ARCHITECTURE.md	Removes PLUGIN_POOL_REQUEST_TIMEOUT_SECS from docs table.
docs/plugins/index.mdx	Updates guidance to per-plugin timeout + HTTP request timeout alignment; removes PLUGIN_POOL_REQUEST_TIMEOUT_SECS.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[NicoMolinaOZ]

LGTM! just one question:

I have one concern here:
Could this change make some pool/server timeouts look like normal plugin timeouts? Since Rust now turns any SocketError with "timed out" into ScriptTimeout, I think a stuck Node pool might return a regular 504 timeout instead of going through the recovery/restart path.
(this is a finding found by Codex analysis :))

[zeljkoX]

LGTM! just one question:

I have one concern here: Could this change make some pool/server timeouts look like normal plugin timeouts? Since Rust now turns any SocketError with "timed out" into ScriptTimeout, I think a stuck Node pool might return a regular 504 timeout instead of going through the recovery/restart path. (this is a finding found by Codex analysis :))

Thanks for feedback.

Sai addressed it already.