Skip to content

Exceptions from tryFinalAcceptance leave the solver active and unrecorded #44

Description

@ugin-man

On main a72b99f60680ffbe56a4c72e113ca52c9469921e, a thrown error from tryFinalAcceptance() escapes step() without setting failed or recording error. The hook runs outside the existing _step() exception handler. A later step() retries the failing hook and increments the iteration count beyond the configured limit.

Reproduction:

import { strict as assert } from "node:assert"
import { BaseSolver } from "./lib/BaseSolver"

class AcceptanceErrorSolver extends BaseSolver {
  MAX_ITERATIONS = 1
  calls = 0
  failure = new Error("final acceptance failed")
  override tryFinalAcceptance() {
    this.calls++
    throw this.failure
  }
}

const solver = new AcceptanceErrorSolver()
assert.throws(() => solver.step(), (error) => error === solver.failure)
console.log(solver.failed, solver.error, solver.iterations) // false, null, 1
assert.throws(() => solver.step(), (error) => error === solver.failure)
console.log(solver.calls, solver.iterations) // 2, 2

The same state inconsistency occurs through solve(). _step() exceptions already set failed/error and rethrow the original exception. Please consider applying a consistent policy to final-acceptance exceptions as well, while retaining successful acceptance and normal exhaustion behavior.

I executed the actual SHA-verified BaseSolver.ts (blob e071bf8c1fe3e2fb4eae73bbaa4e282871d15f02) with Node 22.16.0 type stripping. Three state/retry regressions fail on main; two controls (successful acceptance and normal exhaustion) pass. A local catch-and-record candidate passes all five, but no fix PR is submitted here. Only the test-runner import and explicit .ts import extension were adapted; no solver behavior was mocked.

This differs from #30: that patch guards explicit failed flags and preserves their messages, but does not catch an exception thrown by the acceptance hook. Setup-error PR #37 also leaves final acceptance outside its catch. Reporting separately so any broader hook-error policy can be coordinated instead of stacking another overlapping BaseSolver edit. No full Bun suite or build success is claimed for this report. Prepared with ChatGPT assistance and the account owner's authorization.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions