Code-review fixes: panics, retry, backoff, non-race build, partial detection #1

Merged

kalou merged 0 commits from fixes/code-review into master

2026-09-09 13:17:33 +00:00

kalou commented

2026-09-09 13:12:49 +00:00

(Migrated from git.kalou.net)

Summary

Code-review fixes for the ipfs-search crawler, applied one by one. All build and tests pass locally (go build ./..., go vet ./..., go test ./...).

fix(nsfw): guard getFileStringField against empty/non-string metadata — Tika metadata is untrusted; an empty Content-Type array or non-string first element previously panicked the worker (no recover -> whole pool crash). Now returns "" and logs.
fix(worker): recover panics per-delivery — wraps crawlDelivery with a recover barrier so one bad item can't kill the worker goroutine permanently.
fix(crawl): return error instead of log.Fatal — log.Fatal skipped deferred instFlusher (Jaeger trace flush) and span.End(); now returns the error to the caller.
fix(crawler): seed math/rand — under Go 1.19 (per go.mod) the global source defaults to seed 1, so every worker generated the identical queue-priority sequence. Seeded from the clock in init().
feat(worker): bounded retry for transiently-failing deliveries — previously any crawl error rejected without requeue, permanently dropping items on transient network blips. Classifies errors (transient: request/timeout/context vs permanent), requeues transient ones up to MaxRetries (default 3) via republish with an x-retry-count header, then drops. Adds PublishWithHeaders to the AMQP queue and MaxRetries config plumbed through the pool.
fix(sniffer): exponential backoff on restart — a persistent downstream failure caused a fixed 1s retry loop forever; now uses jittered exponential backoff (1s..60s) and respects context cancellation during the sleep.
fix(docker): build production binary without -race — the runtime image shipped the race detector (2-10x overhead, hard-crash on race). Race tests moved to an optional test stage (docker build --target test).
fix(stat): harden partial-type detection — the unreferenced-partial override fired regardless of stat-resolved type (a 256KB directory could be misclassified) and PartialSize=0 didn't disable it. Now only applies to files, skips when PartialSize==0, and logs when triggered.
docs(readme): note EOL toolchain and forked dependencies — records that Go 1.19, kubo v0.15.0 and opensearch 2.3.0 are past EOL, and that go.mod replaces testify/radix with ipfs-search forks. A version bump is intentionally out of scope here.
chore: remove dead code — drops commented-out OpenTelemetry span scaffolding in sniffer.go and unreachable const debug bool = false log blocks in the index packages.

Test plan

go build ./...
go vet ./...
go test ./... (all packages pass)
docker build --target test -t ipfs-search:test . (race tests, needs live infra for some)
docker build -t ipfs-search . (clean runtime image)

Generated with Devin

## Summary Code-review fixes for the ipfs-search crawler, applied one by one. All build and tests pass locally (`go build ./...`, `go vet ./...`, `go test ./...`). 1. **fix(nsfw): guard `getFileStringField` against empty/non-string metadata** — Tika metadata is untrusted; an empty `Content-Type` array or non-string first element previously panicked the worker (no recover -> whole pool crash). Now returns `""` and logs. 2. **fix(worker): recover panics per-delivery** — wraps `crawlDelivery` with a recover barrier so one bad item can't kill the worker goroutine permanently. 3. **fix(crawl): return error instead of `log.Fatal`** — `log.Fatal` skipped deferred `instFlusher` (Jaeger trace flush) and `span.End()`; now returns the error to the caller. 4. **fix(crawler): seed `math/rand`** — under Go 1.19 (per go.mod) the global source defaults to seed 1, so every worker generated the identical queue-priority sequence. Seeded from the clock in `init()`. 5. **feat(worker): bounded retry for transiently-failing deliveries** — previously any crawl error rejected without requeue, permanently dropping items on transient network blips. Classifies errors (transient: request/timeout/context vs permanent), requeues transient ones up to `MaxRetries` (default 3) via republish with an `x-retry-count` header, then drops. Adds `PublishWithHeaders` to the AMQP queue and `MaxRetries` config plumbed through the pool. 6. **fix(sniffer): exponential backoff on restart** — a persistent downstream failure caused a fixed 1s retry loop forever; now uses jittered exponential backoff (1s..60s) and respects context cancellation during the sleep. 7. **fix(docker): build production binary without `-race`** — the runtime image shipped the race detector (2-10x overhead, hard-crash on race). Race tests moved to an optional `test` stage (`docker build --target test`). 8. **fix(stat): harden partial-type detection** — the unreferenced-partial override fired regardless of stat-resolved type (a 256KB directory could be misclassified) and `PartialSize=0` didn't disable it. Now only applies to files, skips when `PartialSize==0`, and logs when triggered. 9. **docs(readme): note EOL toolchain and forked dependencies** — records that Go 1.19, kubo v0.15.0 and opensearch 2.3.0 are past EOL, and that go.mod replaces testify/radix with ipfs-search forks. A version bump is intentionally out of scope here. 10. **chore: remove dead code** — drops commented-out OpenTelemetry span scaffolding in `sniffer.go` and unreachable `const debug bool = false` log blocks in the index packages. #### Test plan - [x] `go build ./...` - [x] `go vet ./...` - [x] `go test ./...` (all packages pass) - [ ] `docker build --target test -t ipfs-search:test .` (race tests, needs live infra for some) - [ ] `docker build -t ipfs-search .` (clean runtime image) Generated with [Devin](https://devin.ai)

No reviewers

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

kalou/ipfs-search!1

No description provided.

Rows
Columns