-
Notifications
You must be signed in to change notification settings - Fork 52.9k
fix(photon): recover sidecar after outbound transport drops (salvaged v2) #65392
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // Fatal-error helpers for the Photon Spectrum sidecar. | ||
| // | ||
| // Keep this separate from index.mjs so the detection logic can be unit-tested | ||
| // without starting Spectrum or requiring real Photon credentials. | ||
|
|
||
| function appendErrorParts(parts, value, seen) { | ||
| if (value == null) return; | ||
| if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { | ||
| parts.push(String(value)); | ||
| return; | ||
| } | ||
| if (typeof value !== "object") return; | ||
| if (seen.has(value)) return; | ||
| seen.add(value); | ||
|
|
||
| for (const key of ["name", "message", "stack", "code", "grpcCode", "details", "path"]) { | ||
| if (value[key] != null) parts.push(String(value[key])); | ||
| } | ||
| if (value.cause) appendErrorParts(parts, value.cause, seen); | ||
| if (Array.isArray(value.errors)) { | ||
| for (const error of value.errors) appendErrorParts(parts, error, seen); | ||
| } | ||
| } | ||
|
|
||
| export function errorToText(error) { | ||
| const parts = []; | ||
| appendErrorParts(parts, error, new Set()); | ||
| if (!parts.length) return String(error ?? ""); | ||
| return parts.join("\n"); | ||
| } | ||
|
|
||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This classifier is never imported or invoked by |
||
| export function classifyRecoverableOutboundError(error) { | ||
| const text = errorToText(error); | ||
| if (/\[upstream\]\s*Connection dropped/i.test(text)) { | ||
| return "upstream_connection_dropped"; | ||
| } | ||
| return null; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import assert from "node:assert/strict"; | ||
| import test from "node:test"; | ||
|
|
||
| import { classifyRecoverableOutboundError } from "../fatal-errors.mjs"; | ||
|
|
||
| test("classifies upstream connection drops as recoverable outbound send errors", () => { | ||
| const error = new Error("ConnectionError: [upstream] Connection dropped"); | ||
|
|
||
| assert.equal(classifyRecoverableOutboundError(error), "upstream_connection_dropped"); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| import pytest | ||
|
|
||
| from gateway.config import PlatformConfig | ||
| from gateway.platforms.base import SendResult | ||
| from plugins.platforms.photon.adapter import PhotonAdapter | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_direct_send_restarts_sidecar_after_recoverable_upstream_drop(monkeypatch): | ||
| adapter = PhotonAdapter( | ||
| PlatformConfig( | ||
| enabled=True, | ||
| extra={"project_id": "pid", "project_secret": "secret"}, | ||
| ) | ||
| ) | ||
|
|
||
| calls = {"send": 0, "restart": 0} | ||
|
|
||
| async def fake_sidecar_send(chat_id, text): | ||
| calls["send"] += 1 | ||
| assert chat_id == "chat-1" | ||
| assert text == "hello" | ||
| if calls["send"] == 1: | ||
| return SendResult( | ||
| success=False, | ||
| error='Photon sidecar /send returned 500 error_code=upstream_connection_dropped: {"ok":false,"error":"internal sidecar error","error_code":"upstream_connection_dropped"}', | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test injects an |
||
| ) | ||
| return SendResult(success=True, message_id="m-direct-recovered") | ||
|
|
||
| async def fake_restart(reason: str): | ||
| calls["restart"] += 1 | ||
| assert "upstream_connection_dropped" in reason | ||
|
|
||
| monkeypatch.setattr(adapter, "_sidecar_send", fake_sidecar_send) | ||
| monkeypatch.setattr(adapter, "_restart_sidecar_for_outbound_error", fake_restart) | ||
|
|
||
| result = await adapter.send(chat_id="chat-1", content="hello") | ||
|
|
||
| assert result.success is True | ||
| assert result.message_id == "m-direct-recovered" | ||
| assert calls == {"send": 2, "restart": 1} | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_direct_send_recovers_plain_upstream_drop_text(monkeypatch): | ||
| adapter = PhotonAdapter( | ||
| PlatformConfig( | ||
| enabled=True, | ||
| extra={"project_id": "pid", "project_secret": "secret"}, | ||
| ) | ||
| ) | ||
|
|
||
| calls = {"send": 0, "restart": 0} | ||
|
|
||
| async def fake_sidecar_send(chat_id, text): | ||
| calls["send"] += 1 | ||
| if calls["send"] == 1: | ||
| return SendResult( | ||
| success=False, | ||
| error="Photon sidecar /send returned 500: ConnectionError: [upstream] Connection dropped", | ||
| ) | ||
| return SendResult(success=True, message_id="m-plain-text-recovered") | ||
|
|
||
| async def fake_restart(reason: str): | ||
| calls["restart"] += 1 | ||
| assert "upstream_connection_dropped" in reason | ||
|
|
||
| monkeypatch.setattr(adapter, "_sidecar_send", fake_sidecar_send) | ||
| monkeypatch.setattr(adapter, "_restart_sidecar_for_outbound_error", fake_restart) | ||
|
|
||
| result = await adapter.send(chat_id="chat-1", content="hello") | ||
|
|
||
| assert result.success is True | ||
| assert result.message_id == "m-plain-text-recovered" | ||
| assert calls == {"send": 2, "restart": 1} | ||
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_send_with_retry_restarts_sidecar_after_recoverable_upstream_drop(monkeypatch): | ||
| adapter = PhotonAdapter( | ||
| PlatformConfig( | ||
| enabled=True, | ||
| extra={"project_id": "pid", "project_secret": "secret"}, | ||
| ) | ||
| ) | ||
|
|
||
| calls = {"send": 0, "restart": 0} | ||
|
|
||
| async def fake_send(*, chat_id, content, reply_to=None, metadata=None): | ||
| calls["send"] += 1 | ||
| if calls["send"] == 1: | ||
| return SendResult( | ||
| success=False, | ||
| error='Photon sidecar /send returned 500 error_code=upstream_connection_dropped: {"ok":false,"error":"internal sidecar error","error_code":"upstream_connection_dropped"}', | ||
| ) | ||
| return SendResult(success=True, message_id="m-recovered") | ||
|
|
||
| async def fake_restart(reason: str): | ||
| calls["restart"] += 1 | ||
| assert "upstream_connection_dropped" in reason | ||
|
|
||
| monkeypatch.setattr(adapter, "send", fake_send) | ||
| monkeypatch.setattr(adapter, "_restart_sidecar_for_outbound_error", fake_restart) | ||
|
|
||
| result = await adapter._send_with_retry("chat-1", "/new") | ||
|
|
||
| assert result.success is True | ||
| assert result.message_id == "m-recovered" | ||
| assert calls == {"send": 2, "restart": 1} | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please drop this unrelated duplicate pattern. Current
.gitignorealready ignores this glob, then explicitly unignoresglobal.d.tsandvite-env.d.ts; re-adding the glob here after those exceptions changes their precedence.