Repository navigation
fix(ledger): fail import on logs longer than 64 KB - #191
Merged
Merged
Conversation
bufio.Scanner stops at lines over its 64 KB default token size, and the error was only checked inside the loop, where it is always nil. The import stopped silently and still printed "Ledger imported!". Read lines with bufio.Reader.ReadBytes, which has no length limit, in both the import loop and the resume offset lookup. Resume offsets now count actual bytes, and a resume whose last log ID is not in the file fails instead of importing nothing. Confidence: high Scope-risk: narrow
NumaryBot
reviewed
Oct 1, 2026
NumaryBot
added
risk: high
bot-reviewed
review-inconclusive
Bootstrap ./docs (RFC 0004) with a page for fctl ledger import: input format, batching, partial-import behavior, and the new error when --resume-from-last-log cannot find the ledger's last log ID in the file.
NumaryBot
added
risk: high
bot-reviewed
review-approved
NumaryBot
approved these changes
Oct 1, 2026
NumaryBot
left a comment
NumaryBot
left a comment
Contributor
There was a problem hiding this comment.
The required automated review completed with no remaining findings.
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
2 participants
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
fctl ledger importread the file withbufio.Scanner, which stops on any line longer than its 64 KB default. The scanner error was only checked inside the loop, where it is always nil, so a long log ended the import silently: the logs read so far were sent and the command printed "Ledger imported!".bufio.Reader.ReadBytes('\n'), which has no length limit, in both the import loop andopenFileWithOffset(--resume-from-last-log).importLogshelper so read andImportLogserrors are returned and it can be unit tested.+1per line was wrong for CRLF files and for a final line with no newline.--resume-from-last-log, if the ledger's last log ID isn't in the file, the command now fails withlog <id> not found in <file>. Before, it jumped to the end of the file, imported nothing, and reported success.The server side (
internal/api/v2/controllers_logs_import.goin ledger) decodes the body withjson.NewDecoder, so it has no line limit.Docs
Phase 1 of RFC 0004 for this repo: fctl had no
./docs, so this PR addsdocs/README.mdanddocs/ledger-import.md(input format, batching, partial imports, resume, and the new missing-ID error with a Migration note). The docs were AI-drafted from the code and need maintainer review.Test plan
cmd/ledger/import_test.go: a 200 KB log, a final line with no newline,ImportLogserrors returned, resume offset with found and missing IDsgo test ./cmd/ledger/,go vet,golangci-lint run ./cmd/ledger/...(0 issues),go build ./...