From cd1b9b94a4299722ab44ae762aba2ac5e6ab90ba Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 27 Oct 2026 03:10:40 +0100 Subject: [PATCH] Stop asinfo-gen when the as.csv download breaks off (closes #177) parseCSV skipped a record on any read error and asked for the next one. When the download of as.csv broke off, the response body returned the same error on every read, so the generator logged it forever and make asupdate never finished. parseCSV now skips only a record that does not parse, a csv.ParseError. Any other read error ends it with that error, so the generator exits non-zero and script/asupdate keeps the old list. Model: opus-5-5 --- TODO.md | 3 ++ cmd/asinfo-gen/main.go | 11 ++++-- cmd/asinfo-gen/main_test.go | 69 +++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 2 deletions(-) diff --git a/TODO.md b/TODO.md index c23f270..57d039b 100644 --- a/TODO.md +++ b/TODO.md @@ -33,6 +33,9 @@ tested. # Completed Steps +- 2026-10-27: `make asupdate` stops with an error when the download of `as.csv` + breaks off. The generator used to retry the failed read forever. A record that + does not parse is still logged and skipped (closes #177) - 2026-10-27: `make asupdate` runs the new `script/asupdate`. It replaces the AS list built into RouteWatch only when the generator succeeds and finds at least one AS. A failed download used to replace the list with an empty file. The diff --git a/cmd/asinfo-gen/main.go b/cmd/asinfo-gen/main.go index 1b52c19..346c74d 100644 --- a/cmd/asinfo-gen/main.go +++ b/cmd/asinfo-gen/main.go @@ -85,7 +85,8 @@ func main() { // parseCSV reads as.csv from r and returns its ASes, with the handle and the // description as the file spells them. A header whose first columns are not // the AS number, the handle and the description is an error. A record that -// does not parse is logged and skipped. A file left with no AS is an error. +// does not parse is logged and skipped. A read that fails, such as on a broken +// download, is an error. A file left with no AS is an error. func parseCSV(r io.Reader) ([]ASInfo, error) { reader := csv.NewReader(r) @@ -109,12 +110,18 @@ func parseCSV(r io.Reader) ([]ASInfo, error) { break } - if err != nil { + if _, ok := errors.AsType[*csv.ParseError](err); ok { log.Printf("Error reading CSV record: %v", err) continue } + // Any other error comes from the download, such as a broken + // connection. Every later read returns it again. + if err != nil { + return nil, fmt.Errorf("read CSV record: %w", err) + } + asn, err := strconv.Atoi(record[0]) if err != nil { log.Printf("Invalid ASN %q: %v", record[0], err) diff --git a/cmd/asinfo-gen/main_test.go b/cmd/asinfo-gen/main_test.go index 476be04..f629989 100644 --- a/cmd/asinfo-gen/main_test.go +++ b/cmd/asinfo-gen/main_test.go @@ -2,11 +2,18 @@ package main import ( "errors" + "io" "slices" "strings" "testing" + "testing/iotest" + "time" ) +// parseTimeLimit is how long a test waits for parseCSV to return before it +// fails. +const parseTimeLimit = 2 * time.Second + // TestParseCSVReadsTodaysLayout parses an as.csv with the country code column // that the source added after the description. It expects each AS with its // handle and description spelled exactly as in the file, accents included. @@ -71,3 +78,65 @@ func TestParseCSVRejectsAFileWithNoASes(t *testing.T) { } } } + +// TestParseCSVSkipsARecordThatDoesNotParse parses an as.csv with a record +// that has too few fields between two valid records. It expects both valid +// ASes and no error, so that one bad record does not stop make asupdate. +func TestParseCSVSkipsARecordThatDoesNotParse(t *testing.T) { + t.Parallel() + + const asCSV = "asn,handle,description,country-code\n" + + "1,LVLT-1,Level 3 Parent LLC,US\n" + + "2,UDEL-DCN\n" + + "3,MIT-GATEWAYS,Massachusetts Institute of Technology,US\n" + + got, err := parseCSV(strings.NewReader(asCSV)) + if err != nil { + t.Fatalf("parseCSV: %v", err) + } + + want := []ASInfo{ + {ASN: 1, Handle: "LVLT-1", Description: "Level 3 Parent LLC"}, + { + ASN: 3, + Handle: "MIT-GATEWAYS", + Description: "Massachusetts Institute of Technology", + }, + } + + if !slices.Equal(got, want) { + t.Errorf("parseCSV = %+v, want %+v", got, want) + } +} + +// TestParseCSVStopsWhenTheDownloadBreaksOff parses an as.csv whose download +// breaks off after the first record. From then on every read fails, as on a +// broken connection. It expects parseCSV to return that error, so that make +// asupdate keeps the old list. +func TestParseCSVStopsWhenTheDownloadBreaksOff(t *testing.T) { + t.Parallel() + + const asCSV = "asn,handle,description,country-code\n" + + "1,LVLT-1,Level 3 Parent LLC,US\n" + + download := io.MultiReader( + strings.NewReader(asCSV), iotest.ErrReader(io.ErrUnexpectedEOF)) + + // parseCSV runs on its own goroutine, so that a parseCSV that never + // returns fails the test instead of hanging it. + parsed := make(chan error, 1) + + go func() { + _, err := parseCSV(download) + parsed <- err + }() + + select { + case err := <-parsed: + if !errors.Is(err, io.ErrUnexpectedEOF) { + t.Errorf("parseCSV = %v, want %v", err, io.ErrUnexpectedEOF) + } + case <-time.After(parseTimeLimit): + t.Fatalf("parseCSV did not return within %v", parseTimeLimit) + } +}