mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(channels): report a channel that cannot receive as unhealthy instead of connected (#115229)
* fix(channels): make channel health reflect inbound ingress, not just the transport
`connected` is a transport signal, so a channel holding a healthy socket while its
durable ingress queue is unavailable evaluated as `health: healthy` and admitted
nothing. On a production gateway that hid 26+ hours of completely dead inbound Slack
behind `connected, bot:config, app:config, health:healthy`; outbound kept working,
which is what kept the transport looking fine.
Add inbound admission as its own health dimension rather than overloading `connected`
(deliberately tri-state since 30fda068aa, and 17 of 27 channel plugins depend on
absent meaning "no transport signal"). The shared ingress monitor now rethrows a
typed ChannelIngressUnavailableError when it cannot open its durable queue, the
channel supervisor records that as `ingressUnavailable` on the account, and
`evaluateChannelHealth` reports `ingress-unavailable` ahead of every lifecycle
window. Absence stays "unknown", never "fine", so socketless and simply quiet
channels are untouched; no traffic-staleness heuristic is introduced.
The health monitor deliberately does not restart such an account. A denied or
unusable ingress queue is a capability/config failure that a restart cannot fix, and
the channel's own bounded backoff ladder already tried. Today that case instead loops
at the 10-restarts-per-hour cap forever, so this removes a restart treadmill rather
than adding one.
* fix(channels): keep automatic recovery for a dead-ingress account
The health-monitor skip latched a transient queue-open failure into a permanent
inbound outage: ingressUnavailable is only cleared by a later start, and skipping
the restart meant that start never happened. Report the dimension and name the
restart reason instead of suppressing recovery.
* fix(channels): scope the ingress dimension to running accounts
Shadowing not-running would have made readiness flap during the supervisor's
documented restart-backoff grace. The blind spot the dimension exists for is a
running channel with a live transport and dead inbound, so evaluate it there and
leave stopped accounts on their lifecycle reason. Also applies oxfmt.
* fix(channels): keep the ingress reason and give readiness its own backoff grace
Two review cycles pulled in opposite directions on the same bounded window: the
health policy must keep the ingress cause in the not-running state a failed start
actually lands in, while readiness must not flap during restart backoff. Put each
in its owner -- the policy states the truth, and readiness extends its existing
restart-handoff grace to the ingress reason. Once the ladder stops setting
restartPending, the account stays red instead of hiding dead inbound.
* style: reshape the readiness backoff predicate for oxfmt
* fix(ci): regenerate the docs map and drop an unused ingress export
The new health.md section needs docs/docs_map.md regenerated, and knip flagged
CHANNEL_INGRESS_UNAVAILABLE_CODE as an unused production export -- it only has
callers inside its own module.
This commit is contained in:
committed by
GitHub
parent
964c5abbde
commit
58a3051910
@@ -3648,6 +3648,7 @@ Do not edit it by hand; run `pnpm docs:map:gen`.
|
||||
- H2: Quick checks
|
||||
- H2: Deep diagnostics
|
||||
- H2: Health monitor config
|
||||
- H2: Inbound ingress health
|
||||
- H2: Uptime monitoring
|
||||
- H3: Monitoring service setup examples
|
||||
- H2: When something fails
|
||||
|
||||
@@ -42,6 +42,16 @@ health commands above for live connectivity checks.
|
||||
- These per-channel overrides apply to the built-in channels that expose them today: Discord, Google Chat, iMessage, IRC, Microsoft Teams, Signal, Slack, Telegram, and WhatsApp.
|
||||
- A crashing channel is recovered by its own auto-restart backoff first (`auto-restart attempt N/10` in the logs). The health monitor stays out of the way until that ladder ends with `giving up after 10 restart attempts`, then takes over as the last restart owner.
|
||||
|
||||
## Inbound ingress health
|
||||
|
||||
Channel connectivity and inbound admission are separate failure domains. A channel can hold a healthy transport connection — sending replies normally — while its durable ingress queue is unavailable, so not a single inbound message is admitted.
|
||||
|
||||
- When a channel cannot open its durable ingress queue, its start fails and the gateway records the account as unable to receive. `openclaw channels status` reports `Channel cannot admit inbound events; its durable ingress queue is unavailable. Outbound may still work.`
|
||||
- Such an account is **unhealthy** regardless of transport state, and readiness reports it as failing. Previously it reported `health: healthy` and the health monitor never touched it.
|
||||
- Recovery stays automatic. The ingress verdict describes the account's last start attempt and is cleared by the next one, so the ordinary restart path is also how a transient queue-open failure recovers. Those restarts log as `health-monitor: restarting (reason: ingress-unavailable)` instead of the generic `stuck`.
|
||||
- If the restarts keep repeating, the cause is not transient. Check the logged ingress failure: a plugin denied the `openChannelIngressQueue` capability, for example, needs operator action rather than another restart.
|
||||
- Channels that never report ingress state are unaffected: absence means "no signal", never "broken". There is no traffic-staleness heuristic, so a genuinely quiet channel is never marked unhealthy for having received nothing.
|
||||
|
||||
## Uptime monitoring
|
||||
|
||||
External uptime monitoring services should use the dedicated `/health` endpoint, not `/v1/chat/completions`.
|
||||
|
||||
@@ -284,6 +284,9 @@ export function projectSafeChannelAccountSnapshotFields(
|
||||
: {}),
|
||||
...(statusState ? { statusState } : {}),
|
||||
...(healthState ? { healthState } : {}),
|
||||
// Only a proven-dead ingress crosses this boundary; `false`/absent both mean
|
||||
// "nothing known", so never project a negative and invent a healthy claim.
|
||||
...(readBoolean(record, "ingressUnavailable") === true ? { ingressUnavailable: true } : {}),
|
||||
...(readBoolean(record, "terminalDisconnect") !== undefined
|
||||
? { terminalDisconnect: readBoolean(record, "terminalDisconnect") }
|
||||
: {}),
|
||||
|
||||
@@ -8,6 +8,10 @@ import {
|
||||
type ChannelIngressMonitorLifecycle,
|
||||
} from "./ingress-monitor.js";
|
||||
import { createChannelIngressQueue, type ChannelIngressQueue } from "./ingress-queue.js";
|
||||
import {
|
||||
ChannelIngressUnavailableError,
|
||||
isChannelIngressUnavailableError,
|
||||
} from "./ingress-unavailable.js";
|
||||
|
||||
type RawEvent = { id: string; lane: string; text: string };
|
||||
type StoredEvent = { version: 1; rawEvent: string };
|
||||
@@ -504,7 +508,24 @@ describe("channel ingress monitor", () => {
|
||||
const onError = vi.fn();
|
||||
const monitor = createMonitor(queueFactory, vi.fn(), undefined, onError, undefined, 1);
|
||||
|
||||
expect(() => monitor.start()).toThrow(denial);
|
||||
// The typed rethrow is the gateway's only way to tell dead inbound apart from
|
||||
// an ordinary channel crash; the denial stays reachable as the cause.
|
||||
const startError = (() => {
|
||||
try {
|
||||
monitor.start();
|
||||
return expect.unreachable("start must fail while the durable queue is denied");
|
||||
} catch (error) {
|
||||
return error;
|
||||
}
|
||||
})();
|
||||
expect(startError).toBeInstanceOf(ChannelIngressUnavailableError);
|
||||
expect((startError as Error).cause).toBe(denial);
|
||||
expect(isChannelIngressUnavailableError(startError)).toBe(true);
|
||||
// A channel plugin is free to wrap the start failure in its own error.
|
||||
expect(
|
||||
isChannelIngressUnavailableError(new Error("slack start failed", { cause: startError })),
|
||||
).toBe(true);
|
||||
expect(isChannelIngressUnavailableError(denial)).toBe(false);
|
||||
expect(monitor.isRunning()).toBe(false);
|
||||
// An armed poll timer would have retried the denied factory many times over this window.
|
||||
await sleep(25);
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
DEFAULT_INGRESS_RETRY_DEAD_LETTER_MIN_AGE_MS,
|
||||
DEFAULT_INGRESS_RETRY_MAX_ATTEMPTS,
|
||||
} from "./ingress-retry-policy.js";
|
||||
import { ChannelIngressUnavailableError } from "./ingress-unavailable.js";
|
||||
|
||||
const DEFAULT_APPEND_RETRY_DELAYS_MS = [0, 100, 300] as const;
|
||||
|
||||
@@ -603,8 +604,17 @@ export function createChannelIngressMonitor<TRaw, TBody, TStoredPayload, TMetada
|
||||
}
|
||||
// Open the durable queue before arming the poll timer. A monitor without a queue can
|
||||
// neither admit nor drain, so channel start must fail through the caller instead of
|
||||
// running a timer that reports the same unrecoverable error on every tick.
|
||||
getQueue();
|
||||
// running a timer that reports the same unrecoverable error on every tick. The typed
|
||||
// rethrow is what lets the gateway record the failure as dead ingress rather than as
|
||||
// one more anonymous channel crash.
|
||||
try {
|
||||
getQueue();
|
||||
} catch (error) {
|
||||
throw new ChannelIngressUnavailableError(
|
||||
`Channel ingress queue is unavailable: ${formatErrorMessage(error)}`,
|
||||
{ cause: error },
|
||||
);
|
||||
}
|
||||
running = true;
|
||||
pollTimer = setInterval(requestDrain, options.pollIntervalMs);
|
||||
pollTimer.unref?.();
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
/**
|
||||
* Typed marker for "this channel cannot admit a single inbound event".
|
||||
*
|
||||
* Lives in its own module so the gateway supervisor can classify a channel
|
||||
* start failure without importing the whole ingress monitor onto its hot path.
|
||||
*/
|
||||
import { collectErrorGraphCandidates, extractErrorCode } from "../../infra/errors.js";
|
||||
|
||||
const CHANNEL_INGRESS_UNAVAILABLE_CODE = "CHANNEL_INGRESS_UNAVAILABLE";
|
||||
|
||||
/** Raised when a channel's durable ingress queue cannot be opened. */
|
||||
export class ChannelIngressUnavailableError extends Error {
|
||||
readonly code = CHANNEL_INGRESS_UNAVAILABLE_CODE;
|
||||
|
||||
constructor(message: string, options?: { cause?: unknown }) {
|
||||
super(message, options);
|
||||
this.name = "ChannelIngressUnavailableError";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Matches on the stable `code` across the whole `cause` chain rather than
|
||||
* `instanceof`. Channel plugins are free to wrap a start failure in their own
|
||||
* error, and duplicate module instances would defeat a prototype check.
|
||||
*/
|
||||
export function isChannelIngressUnavailableError(error: unknown): boolean {
|
||||
return collectErrorGraphCandidates(error, (current) => [current.cause]).some(
|
||||
(candidate) => extractErrorCode(candidate) === CHANNEL_INGRESS_UNAVAILABLE_CODE,
|
||||
);
|
||||
}
|
||||
@@ -168,6 +168,12 @@ export type ChannelAccountSnapshot = {
|
||||
stateReason?: string;
|
||||
lastError?: string | null;
|
||||
healthState?: string;
|
||||
/**
|
||||
* Inbound admission, which is a different failure domain from `connected`.
|
||||
* Optional-`true` on purpose: there is no `false` to mistake for "unknown",
|
||||
* so the 20+ channels that never report ingress at all stay unaffected.
|
||||
*/
|
||||
ingressUnavailable?: true;
|
||||
terminalDisconnect?: boolean;
|
||||
lastStartAt?: number | null;
|
||||
lastStopAt?: number | null;
|
||||
|
||||
@@ -365,6 +365,27 @@ describe("channel-health-monitor", () => {
|
||||
await expectNoRestart(manager);
|
||||
});
|
||||
|
||||
it("restarts a running channel with a live socket but dead ingress", async () => {
|
||||
// A restart is the only way to re-prove ingress, so recovery from a transient
|
||||
// queue-open failure must stay automatic. Without the ingress dimension this
|
||||
// account evaluated as healthy and was never touched at all.
|
||||
const manager = createSnapshotManager({
|
||||
slack: {
|
||||
default: {
|
||||
running: true,
|
||||
connected: true,
|
||||
enabled: true,
|
||||
configured: true,
|
||||
ingressUnavailable: true,
|
||||
},
|
||||
},
|
||||
});
|
||||
const monitor = await startAndRunCheck(manager);
|
||||
expect(manager.stopChannel).toHaveBeenCalledWith("slack", "default", { manual: false });
|
||||
expect(manager.startChannel).toHaveBeenCalledWith("slack", "default");
|
||||
monitor.stop();
|
||||
});
|
||||
|
||||
it("restarts a stopped channel without terminalDisconnect", async () => {
|
||||
const manager = createSnapshotManager({
|
||||
whatsapp: {
|
||||
|
||||
@@ -263,6 +263,84 @@ describe("evaluateChannelHealth", () => {
|
||||
evaluateHealth({ enabled: true, configured: true, ...snapshot }, { channelId: "whatsapp" }),
|
||||
).toEqual(expected);
|
||||
});
|
||||
|
||||
describe("inbound ingress dimension", () => {
|
||||
it("flags a channel whose ingress monitor failed to start, despite a live transport", () => {
|
||||
// The 26h production case: outbound fine, socket connected, zero inbound admitted.
|
||||
const evaluation = evaluateHealth(
|
||||
connectedAccount({
|
||||
ingressUnavailable: true,
|
||||
lastStartAt: 0,
|
||||
lastTransportActivityAt: 99_000,
|
||||
}),
|
||||
{ channelId: "slack" },
|
||||
);
|
||||
expect(evaluation).toEqual({ healthy: false, reason: "ingress-unavailable" });
|
||||
});
|
||||
|
||||
it("outranks the startup connect grace so dead ingress is never masked", () => {
|
||||
const evaluation = evaluateHealth(
|
||||
connectedAccount({ ingressUnavailable: true, lastStartAt: 99_000 }),
|
||||
{ channelId: "slack" },
|
||||
);
|
||||
expect(evaluation).toEqual({ healthy: false, reason: "ingress-unavailable" });
|
||||
});
|
||||
|
||||
it("keeps the ingress reason for the stopped state a failed start lands in", () => {
|
||||
// server-channels records the verdict only after the start task rejects, so
|
||||
// this is the shape the real failure has. Collapsing it into not-running
|
||||
// would throw the cause away exactly where it matters.
|
||||
const evaluation = evaluateHealth({
|
||||
running: false,
|
||||
enabled: true,
|
||||
configured: true,
|
||||
restartPending: true,
|
||||
ingressUnavailable: true,
|
||||
});
|
||||
expect(evaluation).toEqual({ healthy: false, reason: "ingress-unavailable" });
|
||||
});
|
||||
|
||||
it("outranks a busy short-circuit so an in-flight run cannot hide dead ingress", () => {
|
||||
const evaluation = evaluateHealth(
|
||||
connectedAccount({ ingressUnavailable: true, activeRuns: 1, lastRunActivityAt: 99_000 }),
|
||||
{ channelId: "slack" },
|
||||
);
|
||||
expect(evaluation).toEqual({ healthy: false, reason: "ingress-unavailable" });
|
||||
});
|
||||
|
||||
it("keeps a quiet channel with no traffic and no ingress signal healthy", () => {
|
||||
// Guards against a staleness heuristic sneaking in: a genuinely idle channel
|
||||
// must never be restarted merely for having admitted nothing.
|
||||
const evaluation = evaluateHealth(
|
||||
connectedAccount({
|
||||
lastStartAt: 0,
|
||||
lastEventAt: 0,
|
||||
lastInboundAt: null,
|
||||
lastMessageAt: null,
|
||||
}),
|
||||
{ now: 10_000_000, channelId: "slack" },
|
||||
);
|
||||
expect(evaluation).toEqual({ healthy: true, reason: "healthy" });
|
||||
});
|
||||
|
||||
it("leaves the 17 socketless channels that publish no connectivity untouched", () => {
|
||||
const evaluation = evaluateHealth(runningAccount({ lastStartAt: 0 }), {
|
||||
now: 10_000_000,
|
||||
channelId: "imessage",
|
||||
});
|
||||
expect(evaluation).toEqual({ healthy: true, reason: "healthy" });
|
||||
});
|
||||
|
||||
it("stays healthy for a disabled account so unmanaged still wins", () => {
|
||||
const evaluation = evaluateHealth({
|
||||
running: false,
|
||||
enabled: false,
|
||||
configured: true,
|
||||
ingressUnavailable: true,
|
||||
});
|
||||
expect(evaluation).toEqual({ healthy: true, reason: "unmanaged" });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveChannelRestartReason", () => {
|
||||
@@ -277,6 +355,14 @@ describe("resolveChannelRestartReason", () => {
|
||||
expect(reason).toBe("gave-up");
|
||||
});
|
||||
|
||||
it("maps dead ingress to its own reason instead of stuck", () => {
|
||||
const reason = resolveChannelRestartReason(
|
||||
runningAccount({ connected: true, ingressUnavailable: true }),
|
||||
{ healthy: false, reason: "ingress-unavailable" },
|
||||
);
|
||||
expect(reason).toBe("ingress-unavailable");
|
||||
});
|
||||
|
||||
it("maps disconnected to disconnected instead of stuck", () => {
|
||||
const reason = resolveChannelRestartReason(
|
||||
runningAccount({
|
||||
|
||||
@@ -19,6 +19,7 @@ type ChannelHealthSnapshot = {
|
||||
lastStartAt?: number | null;
|
||||
reconnectAttempts?: number;
|
||||
mode?: string;
|
||||
ingressUnavailable?: true;
|
||||
terminalDisconnect?: boolean;
|
||||
};
|
||||
|
||||
@@ -31,7 +32,8 @@ type ChannelHealthEvaluationReason =
|
||||
| "stuck"
|
||||
| "startup-connect-grace"
|
||||
| "disconnected"
|
||||
| "stale-socket";
|
||||
| "stale-socket"
|
||||
| "ingress-unavailable";
|
||||
|
||||
export type ChannelHealthEvaluation = {
|
||||
healthy: boolean;
|
||||
@@ -45,7 +47,13 @@ export type ChannelHealthPolicy = {
|
||||
channelConnectGraceMs: number;
|
||||
};
|
||||
|
||||
type ChannelRestartReason = "gave-up" | "stopped" | "stale-socket" | "stuck" | "disconnected";
|
||||
type ChannelRestartReason =
|
||||
| "gave-up"
|
||||
| "stopped"
|
||||
| "stale-socket"
|
||||
| "stuck"
|
||||
| "disconnected"
|
||||
| "ingress-unavailable";
|
||||
|
||||
function isManagedAccount(snapshot: ChannelHealthSnapshot): boolean {
|
||||
return snapshot.enabled !== false && snapshot.configured !== false && snapshot.linked !== false;
|
||||
@@ -67,6 +75,15 @@ export function evaluateChannelHealth(
|
||||
if (!snapshot.running && snapshot.terminalDisconnect) {
|
||||
return { healthy: false, reason: "terminal-disconnect" };
|
||||
}
|
||||
// Transport liveness and inbound admission are independent failure domains: a
|
||||
// channel can hold a healthy socket and still admit nothing. This outranks the
|
||||
// lifecycle windows below -- including not-running, which is the state a failed
|
||||
// ingress start actually lands in -- so the cause survives instead of collapsing
|
||||
// into a generic crash. Absence is "unknown", never "fine"; readiness owns its
|
||||
// own restart-backoff tolerance in server/readiness.ts.
|
||||
if (snapshot.ingressUnavailable === true) {
|
||||
return { healthy: false, reason: "ingress-unavailable" };
|
||||
}
|
||||
if (!snapshot.running) {
|
||||
return { healthy: false, reason: "not-running" };
|
||||
}
|
||||
@@ -154,6 +171,12 @@ export function resolveChannelRestartReason(
|
||||
if (evaluation.reason === "stale-socket") {
|
||||
return "stale-socket";
|
||||
}
|
||||
// Restarting is also the only way to re-prove ingress: `ingressUnavailable`
|
||||
// describes the last start attempt and is cleared by the next one. Naming the
|
||||
// reason keeps a repeating restart readable as dead inbound rather than "stuck".
|
||||
if (evaluation.reason === "ingress-unavailable") {
|
||||
return "ingress-unavailable";
|
||||
}
|
||||
if (evaluation.reason === "not-running") {
|
||||
return snapshot.reconnectAttempts && snapshot.reconnectAttempts >= 10 ? "gave-up" : "stopped";
|
||||
}
|
||||
|
||||
@@ -2,11 +2,16 @@
|
||||
* Server channel lifecycle tests.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { ChannelIngressUnavailableError } from "../channels/message/ingress-unavailable.js";
|
||||
import type {
|
||||
ChannelAccountLinkState,
|
||||
ChannelGatewayContext,
|
||||
} from "../channels/plugins/types.adapters.js";
|
||||
import type { ChannelId, ChannelPlugin } from "../channels/plugins/types.public.js";
|
||||
import type {
|
||||
ChannelAccountSnapshot,
|
||||
ChannelId,
|
||||
ChannelPlugin,
|
||||
} from "../channels/plugins/types.public.js";
|
||||
import { formatGatewayChannelsStatusLines } from "../commands/channels/status.js";
|
||||
import type { GatewayNativeApprovalRuntime } from "../infra/approval-gateway-runtime.types.js";
|
||||
import {
|
||||
@@ -88,6 +93,15 @@ type ApprovalGatewayRequestRuntime = Pick<GatewayNativeApprovalRuntime, "request
|
||||
|
||||
const createdManagers: Array<{ manager: ChannelManager; channelIds: ChannelId[] }> = [];
|
||||
|
||||
function healthOf(account: ChannelAccountSnapshot | undefined) {
|
||||
return evaluateChannelHealth(account ?? {}, {
|
||||
channelId: "discord",
|
||||
now: Date.now() + 60 * 60_000,
|
||||
channelConnectGraceMs: 120_000,
|
||||
staleEventThresholdMs: 30 * 60_000,
|
||||
});
|
||||
}
|
||||
|
||||
function createTestPlugin(params?: {
|
||||
id?: ChannelId;
|
||||
order?: number;
|
||||
@@ -331,6 +345,62 @@ describe("server-channels auto restart", () => {
|
||||
expect(startAccount).toHaveBeenCalledTimes(11);
|
||||
});
|
||||
|
||||
it("records dead ingress when a channel start fails to arm its ingress monitor", async () => {
|
||||
const startAccount = vi.fn(async () => {
|
||||
throw new ChannelIngressUnavailableError("Channel ingress queue is unavailable: denied");
|
||||
});
|
||||
installTestRegistry(createTestPlugin({ startAccount }));
|
||||
const manager = createManager();
|
||||
|
||||
await manager.startChannels();
|
||||
const readAccount = () =>
|
||||
manager.getRuntimeSnapshot().channelAccounts.discord?.[DEFAULT_ACCOUNT_ID];
|
||||
await advanceTimersUntil(
|
||||
() => readAccount()?.ingressUnavailable === true,
|
||||
"expected the failed ingress start to be recorded on the account",
|
||||
{ stepMs: 10, maxMs: 500 },
|
||||
);
|
||||
|
||||
// Health must name this dead inbound rather than one more anonymous crash.
|
||||
expect(healthOf(readAccount())).toEqual({
|
||||
healthy: false,
|
||||
reason: "ingress-unavailable",
|
||||
});
|
||||
});
|
||||
|
||||
it("clears a previous lifecycle's dead-ingress verdict once ingress starts again", async () => {
|
||||
let failIngress = true;
|
||||
const startAccount = vi.fn(async () => {
|
||||
if (failIngress) {
|
||||
throw new ChannelIngressUnavailableError("Channel ingress queue is unavailable: denied");
|
||||
}
|
||||
await new Promise(() => {});
|
||||
});
|
||||
installTestRegistry(createTestPlugin({ startAccount }));
|
||||
const manager = createManager();
|
||||
const readAccount = () =>
|
||||
manager.getRuntimeSnapshot().channelAccounts.discord?.[DEFAULT_ACCOUNT_ID];
|
||||
|
||||
await manager.startChannels();
|
||||
await advanceTimersUntil(
|
||||
() => readAccount()?.ingressUnavailable === true,
|
||||
"expected the first start to record dead ingress",
|
||||
{ stepMs: 10, maxMs: 500 },
|
||||
);
|
||||
|
||||
// Runtime rows are patch-merged, so a sticky verdict would keep the channel
|
||||
// unhealthy forever after the operator fixed the underlying capability. The
|
||||
// supervisor's own backoff ladder supplies the next start here.
|
||||
failIngress = false;
|
||||
await advanceTimersUntil(
|
||||
() => readAccount()?.running === true && readAccount()?.ingressUnavailable === undefined,
|
||||
"expected a later start to clear the dead-ingress verdict",
|
||||
{ stepMs: 10, maxMs: 500 },
|
||||
);
|
||||
|
||||
expect(healthOf(readAccount()).reason).not.toBe("ingress-unavailable");
|
||||
});
|
||||
|
||||
it("claims auto-restart ownership between crash-loop attempts", async () => {
|
||||
const startAccount = vi.fn(async () => {});
|
||||
installTestRegistry(createTestPlugin({ startAccount }));
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
// Starts, stops, restarts, and snapshots plugin channel account runtimes.
|
||||
import { RetrySupervisor } from "../../packages/retry/src/index.js";
|
||||
import { getCredentialUnavailableDiagnostics } from "../channels/account-snapshot-fields.js";
|
||||
import { isChannelIngressUnavailableError } from "../channels/message/ingress-unavailable.js";
|
||||
import { resolveChannelDefaultAccountId } from "../channels/plugins/helpers.js";
|
||||
import { type ChannelId, getChannelPlugin, listChannelPlugins } from "../channels/plugins/index.js";
|
||||
import type { ChannelAccountSnapshot } from "../channels/plugins/types.public.js";
|
||||
@@ -709,6 +710,10 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
|
||||
restartPending: false,
|
||||
lastStartAt: Date.now(),
|
||||
lastError: null,
|
||||
// Runtime rows are patch-merged, so a dead-ingress verdict from the
|
||||
// previous lifecycle would outlive the condition it described. Every
|
||||
// start re-proves ingress, so every start must clear it first.
|
||||
ingressUnavailable: undefined,
|
||||
reconnectAttempts: preserveRestartAttempts ? (restarts.get(rKey)?.attempts ?? 0) : 0,
|
||||
});
|
||||
const task = Promise.resolve().then(async () => {
|
||||
@@ -805,7 +810,14 @@ export function createChannelManager(opts: ChannelManagerOptions): ChannelManage
|
||||
return;
|
||||
}
|
||||
const message = formatErrorMessage(err);
|
||||
setRuntime(channelId, id, { accountId: id, lastError: message });
|
||||
setRuntime(channelId, id, {
|
||||
accountId: id,
|
||||
lastError: message,
|
||||
// A channel that never armed its ingress admission is not "crashed":
|
||||
// outbound may work fine while inbound is silently dead. Record the
|
||||
// distinct dimension so health stops reading a live socket as healthy.
|
||||
...(isChannelIngressUnavailableError(err) ? { ingressUnavailable: true } : {}),
|
||||
});
|
||||
log.error?.(`[${id}] channel exited: ${message}`);
|
||||
})
|
||||
.then(async () => {
|
||||
|
||||
@@ -329,6 +329,60 @@ describe("createReadinessChecker", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps a dead-ingress channel ready while its restart backoff is still pending", () => {
|
||||
// The next start re-proves ingress, so this window gets the same grace as any
|
||||
// other restart handoff rather than flapping readiness on every retry.
|
||||
withReadinessClock(() => {
|
||||
const startedAt = Date.now() - FIVE_MIN_MS;
|
||||
const { readiness } = createReadinessHarness({
|
||||
accounts: {
|
||||
discord: managedAccount({
|
||||
running: false,
|
||||
restartPending: true,
|
||||
ingressUnavailable: true,
|
||||
reconnectAttempts: 3,
|
||||
lastStartAt: startedAt - 30_000,
|
||||
lastStopAt: Date.now() - 5_000,
|
||||
}),
|
||||
},
|
||||
});
|
||||
expect(readiness()).toEqual(readySnapshot());
|
||||
});
|
||||
});
|
||||
|
||||
it("fails readiness for dead ingress once the restart ladder stops retrying", () => {
|
||||
withReadinessClock(() => {
|
||||
const { readiness } = createReadinessHarness({
|
||||
accounts: {
|
||||
discord: managedAccount({
|
||||
running: false,
|
||||
restartPending: false,
|
||||
ingressUnavailable: true,
|
||||
reconnectAttempts: 11,
|
||||
}),
|
||||
},
|
||||
});
|
||||
expect(readiness()).toEqual(failingSnapshot(["discord"]));
|
||||
});
|
||||
});
|
||||
|
||||
it("fails readiness for a running channel whose transport is up but ingress is dead", () => {
|
||||
withReadinessClock(() => {
|
||||
const { readiness } = createReadinessHarness({
|
||||
accounts: {
|
||||
discord: managedAccount({
|
||||
running: true,
|
||||
connected: true,
|
||||
restartPending: true,
|
||||
ingressUnavailable: true,
|
||||
lastStartAt: Date.now() - THIRTY_ONE_MIN_MS,
|
||||
}),
|
||||
},
|
||||
});
|
||||
expect(readiness()).toEqual(failingSnapshot(["discord"]));
|
||||
});
|
||||
});
|
||||
|
||||
it("treats stale-socket channels as ready to avoid pulling healthy idle pods", () => {
|
||||
withReadinessClock(() => {
|
||||
const { readiness } = createLongRunningReadinessHarness({
|
||||
|
||||
@@ -38,7 +38,14 @@ function shouldIgnoreReadinessFailure(
|
||||
// Channel restarts spend time in backoff with running=false before the next
|
||||
// lifecycle re-enters startup grace. Keep readiness green during that handoff
|
||||
// window, but still surface hard failures once restart attempts are exhausted.
|
||||
return health.reason === "not-running" && accountSnapshot.restartPending === true;
|
||||
// A failed ingress start lands in the same backoff window, so it gets the same
|
||||
// grace: the next start re-proves ingress, and once the ladder stops setting
|
||||
// restartPending the account stays red instead of hiding dead inbound.
|
||||
const restartableReason =
|
||||
health.reason === "not-running" || health.reason === "ingress-unavailable";
|
||||
const inRestartHandoff =
|
||||
accountSnapshot.restartPending === true && accountSnapshot.running !== true;
|
||||
return restartableReason && inRestartHandoff;
|
||||
}
|
||||
|
||||
/** Create a cached readiness checker over channel runtime health. */
|
||||
|
||||
@@ -143,6 +143,37 @@ describe("collectChannelStatusIssues", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("reports dead ingress even while a restart is pending", () => {
|
||||
mocks.listChannelPlugins.mockReturnValue([createPlugin("slack")]);
|
||||
|
||||
const issues = collectChannelStatusIssues({
|
||||
channelAccounts: {
|
||||
slack: [
|
||||
{
|
||||
accountId: "default",
|
||||
enabled: true,
|
||||
configured: true,
|
||||
running: true,
|
||||
connected: true,
|
||||
restartPending: true,
|
||||
ingressUnavailable: true,
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
expect(issues).toEqual([
|
||||
{
|
||||
channel: "slack",
|
||||
accountId: "default",
|
||||
kind: "runtime",
|
||||
message:
|
||||
"Channel cannot admit inbound events; its durable ingress queue is unavailable. Outbound may still work.",
|
||||
fix: "check openclaw logs for the ingress failure, then rerun openclaw doctor",
|
||||
},
|
||||
]);
|
||||
});
|
||||
|
||||
it("keeps plugin-specific status issues while adding generic runtime issues", () => {
|
||||
const now = Date.now();
|
||||
vi.useFakeTimers();
|
||||
|
||||
@@ -28,6 +28,20 @@ function collectGenericRuntimeStatusIssues(
|
||||
continue;
|
||||
}
|
||||
const accountId = resolveIssueAccountId(account);
|
||||
// Dead ingress outranks the restart-pending short-circuit: a pending restart
|
||||
// cannot fix a channel whose inbound admission is unavailable, and hiding it
|
||||
// behind "status may be stale" is how silent inbound loss stays invisible.
|
||||
if (account.ingressUnavailable === true) {
|
||||
issues.push({
|
||||
channel,
|
||||
accountId,
|
||||
kind: "runtime",
|
||||
message:
|
||||
"Channel cannot admit inbound events; its durable ingress queue is unavailable. Outbound may still work.",
|
||||
fix: "check openclaw logs for the ingress failure, then rerun openclaw doctor",
|
||||
});
|
||||
continue;
|
||||
}
|
||||
if (account.restartPending === true) {
|
||||
issues.push({
|
||||
channel,
|
||||
|
||||
Reference in New Issue
Block a user