mirror of
https://github.com/Studio-Saelix/sencho.git
synced 2026-07-26 11:49:16 +00:00
fix(mesh): name actual bridge kind in proxy-bridge rejection (#1075)
* fix(mesh): distinguish proxy bridge from pilot tunnel in registerProxyBridge rejection When central's MeshProxyTunnelDialer dials a node whose slot is already held by a peer-initiated proxy bridge, the registration is correctly refused, but the activity-log message read "pilot tunnel already registered for node N; proxy bridge refused" regardless of which kind of bridge was actually in the slot. No pilot tunnel exists in proxy-mode remotes by definition, so the wording pointed operators at the wrong subsystem. Use the existing bridgeKinds index (already consulted correctly by the sister method replaceOrRegisterProxyBridge) to emit a kind-accurate message: "proxy bridge already registered for node N; concurrent dial refused" when the existing bridge is a peer-initiated proxy bridge, keeping the original "pilot tunnel" wording for the pilot-agent case so existing log filters and the sister test still match. Pure cosmetic; the rejection behavior is unchanged. New regression test asserts the discriminator in both directions. Also tightens the test-fixture beforeEach to clear bridgeKinds alongside bridges so future tests cannot read a stale kind. * chore: ignore local trailer directory in git repository * refactor(mesh): centralize bridge-kind type and conflict-message formatter Followup tidy on PilotTunnelManager after /simplify flagged two related items in the F-R-2 change: - Extract `BridgeKind = 'pilot' | 'proxy'` as a module-scoped type alias so the kind taxonomy lives in one place. `bridgeKinds` and `injectBridgeForTest` now consume it; the inline union literal is gone. - Extract `formatBridgeConflict(nodeId, existingKind)` as a private helper. Both `registerProxyBridge` and `replaceOrRegisterProxyBridge` call it from their "slot already held" rejection paths, removing the byte-identical "pilot tunnel already registered for node N; proxy bridge refused" string copy across the two sites. - Trim the 6-line WHAT comment above the rejection throw to a 2-line WHY so the call site stays scannable. No behavior change; the three relevant test suites stay green.
This commit is contained in:
@@ -60,6 +60,7 @@ frontend/DESIGN.md
|
||||
|
||||
# Local-only projects (separate repos)
|
||||
demo-video/
|
||||
trailer/
|
||||
website/
|
||||
|
||||
# Internal engineering docs (not published)
|
||||
|
||||
@@ -34,9 +34,13 @@ beforeEach(() => {
|
||||
DatabaseService.getInstance().getDb().prepare("DELETE FROM nodes WHERE name LIKE 'pilot-mgr-test-%'").run();
|
||||
MeshProxyTunnelDialer.resetForTest(0); // idle-close disabled; clears any prior bridges
|
||||
// Drop any bridges held over from a prior test in the same file.
|
||||
// Both maps must clear together: every production write path keeps
|
||||
// `bridges` and `bridgeKinds` in lockstep, and a stale kind entry
|
||||
// would mislead the kind-discriminator in registerProxyBridge.
|
||||
type BridgeMap = Map<number, unknown>;
|
||||
const bridges = (PilotTunnelManager.getInstance() as unknown as { bridges: BridgeMap }).bridges;
|
||||
bridges.clear();
|
||||
const inst = PilotTunnelManager.getInstance() as unknown as { bridges: BridgeMap; bridgeKinds: BridgeMap };
|
||||
inst.bridges.clear();
|
||||
inst.bridgeKinds.clear();
|
||||
});
|
||||
|
||||
function makeFakeBridge(nodeId: number): { bridge: import('../services/PilotTunnelBridge').PilotTunnelBridge; close: () => void } {
|
||||
@@ -91,6 +95,39 @@ describe('PilotTunnelManager.ensureBridge', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('registerProxyBridge rejection names the existing bridge kind (proxy vs pilot)', () => {
|
||||
const mgr = PilotTunnelManager.getInstance();
|
||||
|
||||
// Existing peer-initiated proxy bridge: rejection must say "proxy bridge",
|
||||
// not "pilot tunnel". Locks in the F-R-2 discriminator.
|
||||
const { bridge: existingProxy, close: closeProxy } = makeFakeBridge(404);
|
||||
const { bridge: rejected, close: closeRejected } = makeFakeBridge(404);
|
||||
try {
|
||||
mgr.registerProxyBridge(404, existingProxy);
|
||||
expect(() => mgr.registerProxyBridge(404, rejected))
|
||||
.toThrow(/proxy bridge already registered for node 404/);
|
||||
expect(mgr.getBridge(404)).toBe(existingProxy);
|
||||
} finally {
|
||||
closeProxy();
|
||||
closeRejected();
|
||||
}
|
||||
|
||||
// Existing pilot-agent tunnel: rejection keeps the original "pilot tunnel"
|
||||
// wording so log filters and the sister regex in pilot-tunnel-manager-replace
|
||||
// still match the pilot case.
|
||||
const { bridge: pilot, close: closePilot } = makeFakeBridge(505);
|
||||
const { bridge: rejectedProxy, close: closeRejectedProxy } = makeFakeBridge(505);
|
||||
try {
|
||||
mgr.injectBridgeForTest(505, pilot, 'pilot');
|
||||
expect(() => mgr.registerProxyBridge(505, rejectedProxy))
|
||||
.toThrow(/pilot tunnel already registered for node 505/);
|
||||
expect(mgr.getBridge(505)).toBe(pilot);
|
||||
} finally {
|
||||
closePilot();
|
||||
closeRejectedProxy();
|
||||
}
|
||||
});
|
||||
|
||||
it('registerProxyBridge emits proxy-bridge-up and proxy-bridge-down on lifecycle', async () => {
|
||||
const mgr = PilotTunnelManager.getInstance();
|
||||
const up: number[] = [];
|
||||
|
||||
@@ -33,6 +33,17 @@ export class PilotTunnelCapacityError extends Error {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Discriminator for the two flavors of bridge that share the `bridges`
|
||||
* map: `'pilot'` is set by `registerTunnel` (agent-initiated long-lived
|
||||
* tunnel); `'proxy'` is set by `registerProxyBridge` /
|
||||
* `replaceOrRegisterProxyBridge` (central- or peer-initiated short-lived
|
||||
* proxy bridge). Used by the rejection-message formatter and by
|
||||
* `replaceOrRegisterProxyBridge` so a peer-initiated dial can supersede a
|
||||
* previous proxy bridge but never shadow a live pilot tunnel.
|
||||
*/
|
||||
export type BridgeKind = 'pilot' | 'proxy';
|
||||
|
||||
/**
|
||||
* PilotTunnelManager: singleton registry of active mesh-capable bridges.
|
||||
*
|
||||
@@ -68,15 +79,7 @@ export class PilotTunnelCapacityError extends Error {
|
||||
export class PilotTunnelManager extends EventEmitter {
|
||||
private static instance: PilotTunnelManager;
|
||||
private bridges: Map<number, PilotTunnelBridge> = new Map();
|
||||
/**
|
||||
* Parallel kind index for the `bridges` map. `'pilot'` is set by
|
||||
* `registerTunnel` (agent-initiated long-lived tunnel); `'proxy'` is
|
||||
* set by `registerProxyBridge` and `replaceOrRegisterProxyBridge`
|
||||
* (central-initiated short-lived bridge). Used by
|
||||
* `replaceOrRegisterProxyBridge` so a peer-initiated dial can supersede
|
||||
* a previous proxy bridge but never shadow a live pilot tunnel.
|
||||
*/
|
||||
private bridgeKinds: Map<number, 'pilot' | 'proxy'> = new Map();
|
||||
private bridgeKinds: Map<number, BridgeKind> = new Map();
|
||||
private softWarned = false;
|
||||
|
||||
private constructor() {
|
||||
@@ -111,7 +114,7 @@ export class PilotTunnelManager extends EventEmitter {
|
||||
* Bypasses capacity / lifecycle hooks so unit tests can prime the
|
||||
* registry without owning a real WebSocket.
|
||||
*/
|
||||
public injectBridgeForTest(nodeId: number, bridge: PilotTunnelBridge, kind: 'pilot' | 'proxy'): void {
|
||||
public injectBridgeForTest(nodeId: number, bridge: PilotTunnelBridge, kind: BridgeKind): void {
|
||||
this.bridges.set(nodeId, bridge);
|
||||
this.bridgeKinds.set(nodeId, kind);
|
||||
}
|
||||
@@ -258,10 +261,9 @@ export class PilotTunnelManager extends EventEmitter {
|
||||
public registerProxyBridge(nodeId: number, bridge: PilotTunnelBridge): void {
|
||||
const existing = this.bridges.get(nodeId);
|
||||
if (existing) {
|
||||
// A pilot tunnel for this node already exists. Proxy bridges
|
||||
// should not silently shadow them; refuse the registration so
|
||||
// the dialer can surface a clear error.
|
||||
throw new Error(`pilot tunnel already registered for node ${nodeId}; proxy bridge refused`);
|
||||
// Kind-accurate rejection so the dialer logs the actual conflict
|
||||
// instead of always blaming a pilot tunnel.
|
||||
throw new Error(this.formatBridgeConflict(nodeId, this.bridgeKinds.get(nodeId)));
|
||||
}
|
||||
if (this.bridges.size >= PILOT_TUNNEL_HARD_LIMIT) {
|
||||
PilotMetrics.increment('tunnels_rejected_capacity');
|
||||
@@ -289,7 +291,7 @@ export class PilotTunnelManager extends EventEmitter {
|
||||
public replaceOrRegisterProxyBridge(nodeId: number, bridge: PilotTunnelBridge): void {
|
||||
const existingKind = this.bridgeKinds.get(nodeId);
|
||||
if (existingKind === 'pilot') {
|
||||
throw new Error(`pilot tunnel already registered for node ${nodeId}; proxy bridge refused`);
|
||||
throw new Error(this.formatBridgeConflict(nodeId, existingKind));
|
||||
}
|
||||
if (existingKind === 'proxy') {
|
||||
const old = this.bridges.get(nodeId);
|
||||
@@ -312,6 +314,21 @@ export class PilotTunnelManager extends EventEmitter {
|
||||
this.bridgeKinds.set(nodeId, 'proxy');
|
||||
}
|
||||
|
||||
/**
|
||||
* Single source of truth for the "slot already held" rejection
|
||||
* message thrown by `registerProxyBridge` and
|
||||
* `replaceOrRegisterProxyBridge`. Picks the bridge-kind subject and
|
||||
* the rejection tail so a stale call site cannot drift from the
|
||||
* other. `undefined` collapses to the pilot branch defensively;
|
||||
* production code always sets `bridgeKinds` whenever `bridges` is
|
||||
* set, so this is unreachable in practice.
|
||||
*/
|
||||
private formatBridgeConflict(nodeId: number, existingKind: BridgeKind | undefined): string {
|
||||
return existingKind === 'proxy'
|
||||
? `proxy bridge already registered for node ${nodeId}; concurrent dial refused`
|
||||
: `pilot tunnel already registered for node ${nodeId}; proxy bridge refused`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Force-close a tunnel (e.g., on node deletion).
|
||||
*/
|
||||
|
||||
Reference in New Issue
Block a user