Skip to content

feat: upgrade all deps to latest (Go 1.24), fix bugs/races/leaks, expand test coverage - #11

Merged
thalesfsp merged 1 commit into
mainfrom
claude/upgrade-deps-test-coverage-s9x5bt
Jul 8, 2026
Merged

feat: upgrade all deps to latest (Go 1.24), fix bugs/races/leaks, expand test coverage#11
thalesfsp merged 1 commit into
mainfrom
claude/upgrade-deps-test-coverage-s9x5bt

Conversation

@thalesfsp

Copy link
Copy Markdown
Owner

Summary

Upgrades every dependency to its latest Go 1.24-compatible release, fixes the bugs, data races, and resource leaks found in a full-codebase audit, and adds offline unit/e2e test suites (happy, bad, and edge paths) that run under -race with no external infrastructure.

Dependency upgrades

  • All direct + transitive modules to latest (testify v1.11.1, go-redis v9.21.0, go-elasticsearch v8.19.6, go-sqlite3 v1.14.47, mongo-driver v1.17.9, mysql v1.10.0, pq v1.12.3, sftp v1.13.10, aws-sdk-go v1.55.8, x/crypto v0.48.0, x/text v0.34.0, …).
  • go directive 1.23 → 1.24; CI workflow bumped to Go 1.24 to match.
  • The absolute-latest releases of a few modules (x/crypto v0.53, otel v1.44, x/tools v0.47, …) hard-require Go ≥ 1.25; this PR pins the newest Go 1.24-compatible versions of those instead.
  • Deprecation note: aws-sdk-go v1 and mongo-driver v1 are deprecated upstream in favor of v2 modules with different APIs. Migrating the s3/dynamodb/mongodb storages is a separate, breaking effort (same for go-elasticsearch v9).

Races & leaks fixed

  • Per-adapter package singleton now mutex-guarded (concurrent New/Get/Set raced in all 11 adapters).
  • APM transactions implicitly started by operations were never ended — telemetry silently lost and pooled objects leaked. customapm.Trace now returns a span wrapper whose End() also ends a transaction it created.
  • s3.Retrieve leaked the HTTP response body on every call; s3.Create/Update now use UploadWithContext.
  • redis.New/mongodb.New/sftp.New leaked connection pools, topology goroutines, and SSH sessions when ping/validation failed (mirrors the earlier SQL-adapter fix from fix(sql): close the opened client on New's post-open error paths (connection-handle leak) #10).
  • file.Create with CreateIfNotExist leaked one fd per call; write paths now surface Close errors.
  • Connection-retry loops use retrier.RunCtx, so a canceled context no longer sleeps through ~70s of backoff.

Correctness fixes

  • storage: fan-out helpers dropped zero-valued results (concurrentloop defaults to RemoveZeroValues: true) — a legitimate count of 0, an all-zero document, or an empty generated ID silently vanished; RetrieveFromMany returned only one nondeterministic error; Mock methods panicked (crashing the process from inside worker goroutines) when unset.
  • sql (postgres/sqlite/mysql): target was interpolated verbatim into default Count/List queries — SQL injection; now validated as an identifier. Update of a missing row silently succeeded, now 404 like Retrieve. postgres now uses its registered goqu dialect.
  • elasticsearch: Count passed ListAny by value to a pointer-only assertion, so track_total_hits was dropped and counts silently capped at 10,000; routing condition was inverted (== nil instead of != nil); Query double-counted failures.
  • dynamodb: Count read only the first 1MB scan page (silent undercount) — now paginates with Select: COUNT; expression placeholders are sanitized so attribute names with dashes/dots don't throw ValidationException; List no longer sends invalid Limit: 0; New no longer discards region when a custom config lacks one; vet-flagged non-constant format string fixed.
  • mongodb: List(ctx, target, &out, nil) panicked on nil params; Update matching nothing now returns 404.
  • s3: New called log.Fatal (killing the host process) on session error, and silently returned a cached singleton configured for a different bucket; Retrieve maps NoSuchKey to 404; wrong failure-metric attributions fixed.
  • sftp: the documented host:port address form never worked (url.Parse mangled it); Count overwrote the caller's search pattern and mutated the caller's params struct — now honors the glob without mutation.
  • redis: Count used blocking KEYS (now SCAN) and returned 0 for an empty pattern (now match-all); Update failures incremented the count metric; empty-id Create rejected.
  • memory: Count/List ignored the documented Search glob; Retrieve silently succeeded on non-[]byte values; empty-id Create rejected.
  • file: Create panicked on nil params (deref before nil-check) and ran CreateIfNotExist against the raw arg instead of the resolved Target fallback; Retrieve failures were attributed to the delete metric.
  • internal: customapm.TraceError nil-safe; shared.ErrorContains no longer vacuously matches on an empty expected substring; test HTTP server no longer sleeps 1s per call.

Tests added (all offline, all -race)

  • memory: CRUD round-trip, nil-params, 404s, Search-glob filtering (incl. malformed pattern), wrong-type retrieval, singleton + CRUD concurrency.
  • file: CRUD round-trip, nil-params regression, resolved-target CreateIfNotExist, fd-leak regression (/proc/self/fd), 404 + metric-attribution regressions, glob Count/List, idempotent delete.
  • sqlite (real database, end-to-end): CRUD round-trip, update-missing-row-404, injection-guard (drop/UNION/quote payloads rejected; legit identifiers still work).
  • storage: zero-value preservation across fan-outs, full error aggregation, Mock nil-guards (incl. through a real fan-out).
  • customapm: implicit-transaction lifecycle verified with a recording tracer (ended + reported, caller-owned tx untouched), nil-safety.
  • dynamodb: placeholder sanitization (collision-free), expression builders, error helpers, marshal round-trip.
  • elasticsearch: track_total_hits by pointer and value, from/size emission, JSON validity.
  • sftp: both address forms reach the dialer with the right host.

Existing integration suites (Mongo/Redis/ES/Postgres services) run in CI as before; they couldn't be executed in this sandbox (no Docker daemon), so please let CI validate those paths.

  • Destination branch merged, built and tested with your changes
  • Code formatted and follows best practices and patterns
  • Code builds cleanly (no additional warnings or errors)
  • Manually tested
  • Automated tests are passing (go test -short -race ./...: 15/15 packages; golangci-lint: clean)
  • No decreases in automated test coverage (file 0→64%, memory 0→73%, sqlite 29→64%, storage 46→50%, customapm 0→70%, dynamodb 13→18%)
  • Documentation updated (readme, docs, comments, etc.) — CHANGELOG 2.2.0 entry
  • Localization: No hard-coded error messages in code files (minimally in string constants)

🤖 Generated with Claude Code

https://claude.ai/code/session_01CwjMuEcMQgwWfQF7JbjB6M


Generated by Claude Code

…and tests

Dependencies:
- Upgrade every direct and transitive dependency to its latest Go
  1.24-compatible release; bump go directive 1.23 -> 1.24 and CI to match.

Races and leaks:
- Guard the per-adapter package singleton with a mutex (New/Get/Set raced).
- End implicitly-created APM transactions when the operation span ends
  (they were never ended: telemetry lost, pooled objects leaked).
- s3: close GetObject response body; use UploadWithContext.
- redis/mongodb/sftp New: close client/connection on ping or validation
  failure (connection pools, topology goroutines, SSH sessions leaked).
- file: close the CreateIfNotExist pre-created handle (fd-per-call leak);
  surface write-path Close errors.
- Use retrier.RunCtx so connection retries honor ctx cancellation.

Correctness:
- storage: fan-out helpers no longer drop zero-valued results
  (WithRemoveZeroValues(false)); RetrieveFromMany aggregates all errors;
  Mock methods error instead of panicking when unset.
- sql (postgres/sqlite/mysql): validate target before interpolating into
  default Count/List queries (SQL injection); Update of a missing row is
  404; postgres uses its registered goqu dialect.
- mongodb: List no longer nil-panics on nil params; Update returns 404
  when nothing matched.
- elasticsearch: Count emits track_total_hits again (was capped at 10k
  via a value-vs-pointer assertion); routing condition un-inverted;
  remove double TraceError in Query; drop dead post-defer body check.
- dynamodb: Count paginates LastEvaluatedKey with Select COUNT; sanitize
  expression placeholders (dashes/dots in attribute names); List omits
  invalid Limit 0; New keeps region with a custom config.
- s3: New returns an error instead of log.Fatal; drop stale singleton
  short-circuit (wrong-bucket reuse); Retrieve maps missing keys to 404.
- sftp: accept documented host:port addr; Count honors the search glob
  without mutating caller params.
- redis: Count uses SCAN (not blocking KEYS) and treats empty search as
  match-all; Update failures hit the update (not count) metric.
- memory: Count/List honor the Search glob; Retrieve errors on
  non-[]byte values; Create rejects empty ids (redis too).
- file: Create no longer nil-panics on nil params and resolves the
  Target fallback in CreateIfNotExist; Retrieve metric attribution fixed.
- customapm.TraceError nil-safe; dynamodb vet fix (non-constant format).

Tests:
- New offline suites (happy/bad/edge, all under -race): memory and file
  adapter CRUD + concurrency + fd-leak regression, sqlite end-to-end CRUD
  + injection guard + update-404, storage fan-out zero-value/error
  aggregation + Mock guards, customapm transaction lifecycle via a
  recording tracer, dynamodb expression builders, elasticsearch query
  builder, sftp addr parsing.
- Lint: green with the repo config (usetesting autofixes from the Go 1.24
  bump included); aws-sdk-go v1 deprecation excluded pending a v2
  migration.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CwjMuEcMQgwWfQF7JbjB6M
@thalesfsp
thalesfsp marked this pull request as ready for review July 8, 2026 20:23
@thalesfsp
thalesfsp merged commit a00f71b into main Jul 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants