Conversation
Introduces an optional `on_wire:` kwarg on `Miner#initialize` and `MinerPool#initialize` that takes a Proc of shape `(direction, host, port, payload)` where direction is `:request`, `:response`, or `:response_repaired`. Default `nil` is a no-op, so every existing caller is unaffected. `Miner#query` now builds two requests — the real one sent on the wire and a loggable copy — and the on_wire callback only ever sees the loggable copy. Parameters at known positional indices are redacted in the loggable copy: `addpool` password, `setconfig` value, and `ascset`/`pgaset` values. An integration spec against FakeCgminer's on_request hook pins the important invariant: the wire payload still carries the real secret even when the log shows `[REDACTED]`, so a future refactor cannot silently start shipping the redacted form to cgminer. Library code does not write to stderr. The CLI (next commit) plugs `$stderr` in.
Prints the JSON request and raw response to stderr for wire-level debugging. The formatted result still goes to stdout unchanged, so piping through the CLI is unaffected; the log stream is cleanly separable. Each line carries a `host:port` prefix so operators can `grep` a single miner out of a multi-miner fan-out. Parsed via `OptionParser#permute!` so the flag works before or after the command (`cgminer_api_client -v summary` or `summary -v`). Writes happen inside a closed-over `Mutex` to prevent interleaving when per-miner threads produce responses larger than PIPE_BUF. `DEBUG=1` behavior is unchanged — that env var still only controls the one-shot backtrace on top-level errors.
New CLI usage block under "Exit Codes and Streams" showing the request/response log format, plus a note that addpool/setconfig/ ascset/pgaset secrets are redacted in the log. CHANGELOG gets matching Added entries for both the CLI flag and the `on_wire:` library kwarg.
Three production hazards in the path introduced by PR #3: 1. `@on_wire&.call` propagated callback exceptions out of `Miner#perform_request`, breaking real queries when the operator's callback raised. Route all three call sites through a `safe_on_wire` helper that rescues `StandardError` and swallows — wire logging is best-effort telemetry, not a query precondition. 2. `OptionParser#permute!` raised on unknown flags (e.g. `-x`) with no handler, crashing the CLI before the usage banner printed. Rescue `OptionParser::ParseError`, print the message + USAGE, exit 64. Pre-PR behavior was to pass the unknown flag to cgminer as a positional arg; the new exit-64 path is more conventional. 3. `warn` inside the mutex-wrapped wire-log lambda raised `Errno::EPIPE` / `IOError` when stderr was closed mid-run, aborting the per-miner query thread. Rescue inside the lambda; dropping a log line is strictly better than tearing down the fan-out. Two specs added: an `on_wire` callback that raises (miner keeps returning) and an unknown-flag CLI invocation (exit 64 + usage).
Three behaviors that the existing suite would have silently let a
refactor break:
- :response_repaired: no spec forced the `}{` / `[,{` gsub to actually
mutate the response, so the callback emission at
miner.rb:113-117 and the CLI's `wire_prefix[:response_repaired]`
lookup were both untested. Added a unit spec that feeds a
`{"foo":[{"a":1}{"b":2}]}` fixture through a stubbed socket and
asserts both callbacks fire with the right payload shapes.
- :response with control bytes: the existing `ok_response` has no
bytes < 0x20, so pre- and post-escape were identical and a refactor
that moved the callback above the `chars.map` escape would pass.
Added a spec asserting the callback payload contains the
`\uXXXX`-escaped form, not the raw byte.
- MinerPool non-nil on_wire pass-through: the pool spec only covered
the nil-default case. A wrapping refactor could silently give each
Miner a distinct wrapper, breaking the CLI mutex's pool-wide scope.
Added an identity-pass-through spec.
- The mutex rationale in bin/cgminer_api_client cited PIPE_BUF as the reason writes could tear mid-line. PIPE_BUF atomicity applies to pipes/FIFOs, not stderr generally; the real reason is that Ruby's `warn` / `$stderr.write` isn't atomic across threads for arbitrary-length payloads. Reword the comment to teach the right invariant. - The on_wire describe block in miner_spec.rb claimed "perform_request is stubbed." It isn't — stub_wire only stubs open_socket, and perform_request runs end-to-end. That distinction matters when debugging failures (a breakpoint in perform_request actually hits). Reword to reflect reality. - One example was named "redacts when invoked via method_missing sugar" but addpool is defined via Privileged::Pool#addpool — method_missing is never triggered. Rename and rephrase the comment so a reader isn't sent hunting for a method_missing path that doesn't exist. Also add a one-liner to REDACTED_PARAM_INDEX's docstring reminding future contributors to register new secret-bearing admin commands.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
-v/--verboseflag tobin/cgminer_api_clientthat prints the JSON request and raw response to stderr before the formatted result goes to stdout. Removes the "edit the binary to add a puts" debug ritual for operators trying to understand why a cgminer verb isn't behaving.Miner#initializeandMinerPool#initializeget an optionalon_wire:kwarg — aProcwith signature(direction, host, port, payload). Defaultnilis a no-op, so every existing caller is unaffected. Library code itself never writes to stderr (honoring the AGENTS.md rule).$stderrin aMutexand provides the callback when-vis set.MinerPool's per-miner thread fan-out can produce responses larger than PIPE_BUF (4 KiB on Linux, 512 B on macOS), so the mutex keeps each request/response pair contiguous. Every line is prefixed withhost:portso operators cangrepa single miner out of the mixed stream.OptionParser#permute!, so-v summary,--verbose summary, andsummary -vall work.addpool, and values insetconfig,ascset,pgasetare replaced with[REDACTED]in the log output. An integration spec againstFakeCgminer'son_request:hook pins the invariant: the wire payload still carries the real secret, so a future refactor can't silently start shipping[REDACTED]to cgminer.DEBUG=1behavior is unchanged — still controls the one-shot backtrace on top-level errors only.Test plan
bundle exec rake(rspec + rubocop) — 221 examples, 0 failures, 0 offenseson_wirecallback, redaction across all four commands,method_missingpath, escape-under-redaction, and theon_wire: nildefault[REDACTED]-v <cmd>,--verbose <cmd>,<cmd> -v(permute), silent stderr without the flag, and per-miner prefix on multi-miner fan-outCommits
8a7fbbeAdd on_wire callback to Miner and MinerPool with secret redactiond61616cAdd -v/--verbose flag to the CLIbb50148Document --verbose in README and CHANGELOG