Skip to content

miner(manage): a local-store open failure escapes runManageStatus/runManagePoll as an unhandled throw and leaks the sibling handle #10003

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

packages/loopover-miner/docs/unattended-scheduling.md:16-23 defines the exit-code contract for exactly these two
commands:

Exit code Meaning
0 Success — the command completed.
2 Failure — invalid arguments, or the run hit an error (network / API / local state). Alert on this.

bin/loopover-miner.ts:43-50 reiterates it for its own _FILE secret handling: "Exits 2, not 1:
docs/unattended-scheduling.md's contract only defines 0 (success) and 2 (failure -- 'Alert on this'), so an
operator alerting strictly on 2 would otherwise miss a broken secret mount entirely."

Both manage entry points open their SQLite stores outside the try/finally that is supposed to honour that
contract.

packages/loopover-miner/lib/manage-status.ts:278-299:

  const ownsPortfolioQueue = options.initPortfolioQueue === undefined;
  const ownsEventLedger = options.initEventLedger === undefined;
  const ownsRunStateStore = options.initRunStateStore === undefined;
  const portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)();
  const eventLedger = (options.initEventLedger ?? initEventLedger)();
  const runStateStore = (options.initRunStateStore ?? initRunStateStore)();
  try {
    ...
  } catch (error) {
    // Collecting/rendering manage status touches three SQLite stores; a read/render failure must surface as a
    // clean CLI error (honoring --json), not an unhandled throw -- matching runOrbExportCli / runQueueList (#7236).
    return reportCliFailure(parsed.json, describeCliError(error));
  } finally {
    if (ownsPortfolioQueue) portfolioQueue.close();

packages/loopover-miner/lib/manage-poll.ts:303-341 has the identical shape:

  const ownsEventLedger = options.initEventLedger === undefined;
  const ownsPortfolioQueue = options.initPortfolioQueue === undefined;
  const eventLedger = (options.initEventLedger ?? initEventLedger)();
  const portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)();

  try {

Opening a store is a real failure mode, not a hypothetical: openLocalStoreDb
(packages/loopover-miner/lib/local-store.ts:66-88) does mkdirSync, new DatabaseSync(path) and chmodSync,
and initEventLedger additionally runs CREATE TABLE, applySchemaMigrations and pruneLedgerByRetention
(packages/loopover-miner/lib/event-ledger.ts:160-178). A corrupt file, a read-only mount, an unwritable state
dir, or a schema migration that throws all raise here. doctor ships a whole store-integrity:* sweep
(packages/loopover-miner/lib/status.ts:392-429) precisely because these files do get corrupted.

Two things go wrong when that happens:

  1. The exit code violates the documented contract. bin/loopover-miner.ts:120 calls
    process.exit(runManageStatus(cliArgs.slice(2))) and :223 awaits runManagePoll(...); neither the bin nor
    these functions has a surrounding catch, so the throw becomes an uncaught exception/unhandled rejection and
    Node exits 1 with a stack trace — not the 2 the docs tell operators to alert on. An unattended cron/systemd
    job matching the documented contract records the failure as "not the alertable kind", and --json consumers
    get a stack trace on stderr instead of { ok: false, error }.
  2. A handle leaks. In manage-status.ts, if initEventLedger() throws, the already-open portfolioQueue is
    never closed — the finally belongs to a try that was never entered. Same for manage-poll.ts when
    initPortfolioQueueStore() throws after initEventLedger() succeeded.

The in-file precedent for the correct shape is runDiscover, at
packages/loopover-miner/lib/discover-cli.ts:631-637:

  const ownsPortfolioQueue = options.initPortfolioQueue === undefined;
  let portfolioQueue: PortfolioQueueStore | undefined;
  try {
    portfolioQueue = (options.initPortfolioQueue ?? initPortfolioQueueStore)();
  } catch (error) {
    return reportCliFailure(parsed.json, describeCliError(error));
  }

and its finally guards each close with if (owns… && handle) (discover-cli.ts:768-773).

The existing #7236 regression test only covers a store whose method throws, not one whose opener throws —
test/unit/miner-manage-status.test.ts:327-336 injects initPortfolioQueue: () => throwingQueue where
throwingQueue.listQueue() throws, so the failure happens inside the try.

Requirements

  • runManageStatus and runManagePoll must return 2 (via reportCliFailure, honouring --json) when any of
    their store openers throws — never propagate the throw to the caller.
  • Every store handle already opened when a later opener throws must be closed before returning, and only if this
    function owns it (options.init… === undefined).
  • The --json failure payload must be the existing { ok: false, error: <message> } shape produced by
    reportCliFailure (packages/loopover-miner/lib/cli-error.ts:4-11), with the message from
    describeCliError.
  • The success paths, the argument-parse failure paths, and the injected-store ownership semantics
    (ownsPortfolioQueue / ownsEventLedger / ownsRunStateStore — an injected store is never closed by these
    functions) must be unchanged.
  • The #7236 behaviour — a throw from listQueue/readEvents/listRunStates inside the try still returning 2
    — must keep working.

⚠️ Required pattern: mirror packages/loopover-miner/lib/discover-cli.ts:631-637 and its guarded finally at
:768-773 exactly — per-opener try/catch returning reportCliFailure, nullable handle variables, and
if (owns… && handle) handle.close() in the finally. What does NOT satisfy this issue: (a) wrapping the whole
function body in one big try/catch that still leaks the sibling handle because the openers ran before any
handle variable was assigned; (b) adding a top-level process.on("uncaughtException") handler in
bin/loopover-miner.ts, which changes process-wide behaviour for every subcommand; (c) fixing only
manage-status.ts and leaving manage-poll.ts — the doc's exit-code table names both commands.

Deliverables

  • runManageStatus([], { initPortfolioQueue: () => { throw new Error("boom"); }, … }) returns 2 and prints
    boom to stderr; with ["--json"] it prints { ok: false, error: "boom" } — asserted in
    test/unit/miner-manage-status.test.ts.
  • runManageStatus with a working initPortfolioQueue and a throwing initEventLedger returns 2 and
    closes the portfolio queue exactly once — asserted with a spy close on the injected store in
    test/unit/miner-manage-status.test.ts. (Ownership caveat: use a real owned store via
    LOOPOVER_MINER_CONFIG_DIR or an injected store plus an explicit close assertion consistent with the
    ownership rule the implementation keeps.)
  • await runManagePoll(["acme/widgets", "4"], { initEventLedger: () => { throw new Error("boom"); }, … })
    returns 2, and with --json prints { ok: false, error: "boom" } — asserted in
    test/unit/miner-manage-poll.test.ts.
  • await runManagePoll(["acme/widgets", "4"], …) with a working initEventLedger and a throwing
    initPortfolioQueue returns 2 and the event ledger is closed — asserted in
    test/unit/miner-manage-poll.test.ts.
  • A regression test in each of test/unit/miner-manage-status.test.ts and
    test/unit/miner-manage-poll.test.ts named for this bug (e.g. REGRESSION: a store-open failure exits 2 instead of throwing, and closes the handle already opened) that fails against the current code.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one that
makes both commands return 2 but still leaks the first-opened handle when the second opener throws — does not
resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's coverage.include lists
packages/loopover-miner/lib/**/*.ts, so manage-status.ts and manage-poll.ts are measured and gated. Every
branch the change introduces needs both arms tested: each new opener try/catch (success and throw), each
if (owns… && handle) guard in the finally (handle present and handle still undefined), and the existing
parsed.json true/false split on the failure path.

Expected Outcome

A corrupt or unopenable local store makes loopover-miner manage status and loopover-miner manage poll exit
with the documented failure code 2 and a parseable --json error object, instead of crashing with an uncaught
exception and exit code 1 that an operator alerting on the documented contract would classify as the wrong kind
of failure — and no SQLite handle is left open when one of several openers fails.

Links & Resources

  • packages/loopover-miner/docs/unattended-scheduling.md:16-23 — the exit-code contract for these two commands
  • packages/loopover-miner/lib/manage-status.ts:278-303 — the openers outside the try/finally
  • packages/loopover-miner/lib/manage-poll.ts:303-342 — the same shape
  • packages/loopover-miner/lib/discover-cli.ts:631-637, :768-773 — the correct pattern to mirror
  • packages/loopover-miner/lib/local-store.ts:66-88, packages/loopover-miner/lib/event-ledger.ts:160-178 — what
    an open actually does
  • test/unit/miner-manage-status.test.ts:327-350 — the #7236 test that only covers an in-try throw

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions