HarperFast / HarperFast/harper-pro
Add fault-injection hook for in-replicateOverWS close() paths in integration tests
- Dominant language
- JavaScript
- Stars
- 3
- Forks
- 0
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 80
Description
## Summary
The new tests in `integrationTests/cluster/replicationReconnect.test.mjs` (introduced by https://github.com/HarperFast/harper-pro/pull/154) cover the user-driven `unsubscribe()` teardown and the generic transient retry path (TCP close after `killHarper`). They do **not** cover the protocol-level `close(...)` paths inside `replicateOverWS`, which were the actual regression that #154 fixes:
- peer-initiated `DISCONNECT` (line 622-623)
- `close(1008, 'Unauthorized')` after WS open (500, 514, 1033, 1044)
- `close(1008, 'Node name mismatch')` (544)
- `close(1008, 'Invalid sequence id ...')` (1416)
- `close(1008, 'Error handling subscription to node ...')` (1550)
- the 60s empty-subscription delayed close (2113)
These all formerly set `ws.isFinished = true` and emit `'finished'`, killing the connection silently with no retry. The fix changes that for everything except the 60s delayed close. Without a way to trigger one of these closes from an integration test, the specific regression isn't directly guarded.
## What's needed
A way for an integration test to deterministically cause one of these closes on a live replication connection. Some options, in increasing order of intrusiveness:
1. **Test-only operation** that injects a fault on the next replication message from a specific peer (e.g., flips the next received command id to an unknown one, triggering `close()` at line 1389). Gated behind an env var so it can't ship in prod.
2. **Hook in `replicateOverWS`** that takes an optional `_testHooks` object on the options bag, allowing tests to override behavior (return a fake `setDatabase`, force a NODE_NAME mismatch, etc.). Slightly more invasive but more flexible.
3. **A bare WS client helper** that knows the `harperdb-replication-v1` subprotocol enough to perform the NODE_NAME handshake and then send a raw `[142]` (DISCONNECT) command. Use it as the "peer" in a test where the real Harper node is the outgoing client — verify the outgoing client retries instead of silently dying. Closest to a real-world repro and doesn't touch prod code.
Option 3 is probably the right call — it doesn't require any production-code changes and reproduces the exact bug shape.
## Acceptance
Once the helper exists, extend `replicationReconnect.test.mjs` (or add a sibling) with at least one test that:
1. Stands up a real Harper node.
2. Connects to its replication endpoint as a fake peer, completes the NODE_NAME handshake.
3. Sends a `DISCONNECT` command frame.
4. Asserts the real node logs a transient close, schedules a retry, and successfully re-handshakes within a bounded time window.
## Why now
Pro currently has no unit-test runner (`AGENTS.md` is explicit; only `npm run test:integration` exists), and the existing integration harness has no fault-injection. The bug fixed in #154 took live CDP debugging to find — having a repro in CI would have caught it years earlier.
Filed as follow-up from https://github.com/HarperFast/harper-pro/pull/154.
Contributor guide
Assessment
This issue has not been assessed yet.