fix(photon): prevent sidecar death spiral with health-check server and restart counter reset - #53197
fix(photon): prevent sidecar death spiral with health-check server and restart counter reset#53197cjboy007 wants to merge 2 commits into
Conversation
The Feishu/Lark file upload API returns error 99992402 when the multipart file field lacks a Content-Type header. This fix uses mimetypes.guess_type() to infer the MIME type from the filename and passes it as a (filename, file, mime_type) tuple to requests, falling back to application/octet-stream when the type is unknown.
…d restart counter reset Fixes NousResearch#49858 Root cause: 5 compounding bugs created a permanent death spiral: 1. Sidecar initialization blocks on rate limit before opening port - Spectrum() init could hang on 429, but HTTP server was defined after - Adapter checks port, finds nothing, kills sidecar, retries - Creates kill loop that burns through rate limit quota faster 2. Adapter restart counter gives up permanently after 5 attempts - Once _RESTART_MAX_ATTEMPTS exhausted, never retries - No recovery without manual gateway restart Fixes: - Start temporary health-check HTTP server on port 8789 FIRST - Then initialize Spectrum() asynchronously with 429 retry logic (wait 60s, max 5 attempts) - Swap to full server after successful init - Reset restart counter after 30 min stability - Retry after 5 min cooldown instead of permanent give-up This ensures adapter can detect sidecar is alive even during slow init, and prevents permanent lockout after transient failures.
alt-glitch
added
type/bug
teknium1
left a comment
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the detailed Photon recovery investigation. Current main already handles the sidecar-exit portion through gateway-managed retry (plugins/platforms/photon/adapter.py:1022-1034, gateway/run.py:7841-7925), but the remaining initialization-rate-limit idea needs rework.
Problems
plugins/platforms/photon/sidecar/index.mjs:145exposesGET /health, while the unchanged readiness probe sends authenticatedPOST /healthzand accepts only 200 (plugins/platforms/photon/adapter.py:831-836). The temporary server returns 503, so it cannot keep startup alive during initialization.- The 30-minute reset cannot run: timestamps older than 600 seconds are pruned at
plugins/platforms/photon/adapter.py:887-890before the> 1800check at:894-902. - The diff adds no regression tests for the 429, readiness, restart, or Feishu MIME paths.
Suggested changes
- Rework the Photon change on current main around the existing gateway reconnect owner, and make any temporary endpoint match the actual authenticated
/healthzreadiness contract. - Add deterministic tests for initialization retries and stable-runtime reset semantics; split and test the unrelated Feishu upload change separately.
Automated hermes-sweeper review.
|
|
||
| // Temporary health-check server (just responds to /health) | ||
| const healthServer = http.createServer((req, res) => { | ||
| if (req.url === '/health') { |
There was a problem hiding this comment.
The adapter still probes authenticated POST /healthz and accepts only HTTP 200 (adapter.py:831-836), but this temporary server returns 200 only for /health. During initialization it will return 503 to every readiness probe, so the proposed server does not prevent the 15-second startup failure.
| now = time.time() | ||
|
|
||
| # Prune old timestamps outside the window | ||
| self._restart_timestamps = [ |
There was a problem hiding this comment.
This pruning removes all timestamps older than 10 minutes before the later stable_duration > 1800 check. Any timestamp that survives is necessarily newer than 10 minutes, so the advertised 30-minute stability-reset branch is unreachable.
teknium1
added
sweeper:risk-compatibility
|
Closing — sidecar-exit recovery is gateway-owned on current main (supervisor + reconnect queue), the unauthenticated GET /health duplicated the authenticated POST /healthz probe, and the 30-min restart counter reset was unreachable (pruned at 600s). The problem you targeted was real; the mechanism landed differently via #73563. Thanks for the thorough attempt. |
3 participants
Summary
Fixes #49858
Root Cause
Two bugs in the Photon adapter and sidecar caused a death spiral when the sidecar crashed:
Spectrum()initialization had no error handling for rate limits. If Photon returned 429, the sidecar exited immediately, and the adapter couldn't detect it was alive (port not yet open).Changes
adapter.py — stop giving up permanently
sidecar/index.mjs — survive rate limits during init
Spectrum()initSpectrum()asynchronously with 429 retry logic (wait 60s, max 5 attempts)Key insight
The sidecar's
for(;;)reconnection loop inindex.mjswas already correct. The bugs were in the adapter's restart logic and the sidecar's init sequence, which prevented proper recovery from transient failures.