#36210 streams: Include the OS error when AutoFile I/O fails

ReviewabilityReviewsAgreementSize
#36210 pablomartin4btc streams: Include the OS error when AutoFile I/O failsReady0Positive14 + 104 tests
  • Include specific operating system error details when file stream operations fail
  • Helps users and callers diagnose root causes such as full disks, invalid seeks, or permission errors
  • Ready to review
  • Concept approval without stated reasons (w0xlt)
  • Resolved objection: description and comments reworded to follow project AI policy (maflcko)
  • 14 lines added or modified outside tests
  • 104 lines added or modified in tests
  • 6 lines removed in total
  • 3 files, 2 commits
Utilities (logging, arguments, libraries)#43 of 59
P3 · bug fix
  • P3 because callers previously had no way to distinguish out-of-space or permission errors from generic failures
  • Improves diagnostic value across stream operations with low risk and self-contained changes
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Captures errno immediately after failed fread, fwrite, and fseek calls in AutoFile and BufferedFile, appending the formatted OS error string via SysErrorString to thrown std::ios_base::failure exceptions. Adds unit test coverage asserting that genuine I/O failures and invalid seeks propagate the OS error.

Problem: AutoFile and BufferedFile throw static, generic error messages on non-EOF I/O failures, discarding errno. Callers displaying or logging exception messages cannot determine why a file operation failed (such as out-of-space, permissions, or invalid seek).

Evidence of importance:

  • PR description: author observed missing OS error details while reviewing PR #35492, which transitions wallet dumping to AutoFile, leaving callers unable to distinguish causes like disk full or permission errors from generic write failures.

Risk of a mistake: low (tests, other). Only modifies exception strings thrown on already-failing file I/O operations in utility stream classes.

Changed since the previous assessment: Includes AutoFile::seek and fseek failures in addition to fread/fwrite, adding test coverage for seeking before the start of a file.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: review of reworded description/comment and updated changes 2026-10-29

Agreement

Positive

support without stated reasons, no open objection (w0xlt)

Thread notes: The author reworded the PR description and previous comment to comply with the AI policy and rebased with added seek support.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)objection, maintenancePR description and comments violate the project AI policyfixed · fix pushed · author reworded description and comment as requestedchangedyes2026-10-22: 'Can you please write the pull description and any comments without using an LLM? see https://github.com/bitcoin/bitcoin/blob/master/doc/AI_POLICY.md'
Status from: 2026-10-29: pablomartin4btc: 'Reworded PR description and a [comment](https://github.com/bitcoin/bitcoin/pull/36210#issuecomment-5611987438) as [requested](https://github.com/bitcoin/bitcoin/pull/36210#issuecomment-6343625652) by @maflcko.'

Support:

  • w0xlt (contributor) (Concept ACK, 2026-09-22): (no reason given) [not substantive]

Participants:

  • w0xlt (contributor, support, 1 statements 2026-09-22 to 2026-09-22): Concept ACK
  • maflcko (member, objection, 1 statements 2026-10-22 to 2026-10-22): asked to rewrite description and comments without LLMs per AI policy

Dependencies

Enables:

Files

104 lines under test/bench/ci.

  • src/test/streams_tests.cpp +104/-0
  • src/streams.cpp +11/-5
  • src/streams.h +3/-1

Card

The short text the ranking pass compares across PRs.

This pull request updates AutoFile and BufferedFile to capture errno on failing fread, fwrite, and fseek calls, appending the OS error message via SysErrorString to thrown std::ios_base::failure exceptions. Currently, I/O failures throw generic strings that discard the operating system error, leaving callers unable to diagnose causes like disk full or permission denied. The author encountered the issue while reviewing wallet dump improvements in #35492. It includes cross-platform unit tests verifying error propagation.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-31T01:14, confidence high, input hash 0617455ff23515fb
thread read through 2026-10-29 (5 statements), head 5686bd642a
labels: none

Categories

Utilities (logging, arguments, libraries)

P3 · bug fix

Improves diagnostic value of stream I/O exceptions by attaching the operating system error string via SysErrorString instead of discarding errno. Callers previously had no way to distinguish out-of-space, permissions, or invalid seek failures from generic write errors.

Membership: Modifies core stream primitives in src/streams.cpp and src/streams.h to preserve OS error details upon I/O failure.

Factors: security/stability 0, bug 1, performance 0, user value 1, leverage 1