From 395e9799b8a3278e69ccfefae4e215fcbbfc335d Mon Sep 17 00:00:00 2001 From: Brandur Date: Sat, 26 Apr 2025 12:44:09 -0700 Subject: [PATCH] Add `nilerror` hook to detect nil structs wrapped in non-nil error interfaces This one's aimed at helping people debug problems like the one raised here [1]. There's a common mistake in Go where a nil error-compliant struct is wrapped in a non-nil error interface and unexpectedly caught by a `if err != nil { ... }` check. For example: type CustomError { InternalErr error } type JobWorker struct { river.WorkerDefaults[workers.JobWorkerArgs } func (jw *JobWorker) Work(ctx context.Context, job *river.Job[workers.JobWorkerArgs]) error { var customErr *CustomError return customErr } This is such a common problem in Go that the language maintains a long FAQ on the topic: https://go.dev/doc/faq#nil_error We probably don't want to bake a solution for this right into core River because there'd be huge room for disagreement for what to do about it. Purists would not want the case handled because technically Go is behaving the way that it's supposed to. Beyond that, in case a problem is found, it's debatable whether an error should be emitted (so it can be easily caught in tests) or just logged. Here, we add a hook in `rivercontrib` as a nice compromise. People who want extra checks on this problem can easily configure it, and everyone else can ignore it. This requires a new `HookWorkEnd` interface in core River to work [2]. It could've been added as a middleware instead, but it seems to me that a lighter check that doesn't go on the stack in a hook is the better approach. [1] https://github.com/riverqueue/river/issues/806 [2] https://github.com/riverqueue/river/pull/863 --- CHANGELOG.md | 6 +- datadogriver/go.mod | 16 ++--- datadogriver/go.sum | 20 ++++-- go.work | 3 +- nilerror/README.md | 19 ++++++ nilerror/example_hook_test.go | 31 ++++++++++ nilerror/go.mod | 30 +++++++++ nilerror/go.sum | 66 ++++++++++++++++++++ nilerror/hook.go | 76 +++++++++++++++++++++++ nilerror/hook_test.go | 92 ++++++++++++++++++++++++++++ otelriver/example_middleware_test.go | 2 +- otelriver/go.mod | 4 +- 12 files changed, 350 insertions(+), 15 deletions(-) create mode 100644 nilerror/README.md create mode 100644 nilerror/example_hook_test.go create mode 100644 nilerror/go.mod create mode 100644 nilerror/go.sum create mode 100644 nilerror/hook.go create mode 100644 nilerror/hook_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index e072681..f4474cc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,11 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `nilerror` hook to detect nil structs wrapped in non-nil error interfaces. [PR #25](https://github.com/riverqueue/rivercontrib/pull/25). + ## [0.4.0] - 2025-04-18 ### Added -- Added `otelriver` option `EnableWorkSpanJobKindSuffix` which appends the job kind a suffix to work spans so they look like `river.work/my_job` instead of `river.work`. +- Added `otelriver` option `EnableWorkSpanJobKindSuffix` which appends the job kind a suffix to work spans so they look like `river.work/my_job` instead of `river.work`. [PR #23](https://github.com/riverqueue/rivercontrib/pull/23). ## [0.3.0] - 2025-04-14 diff --git a/datadogriver/go.mod b/datadogriver/go.mod index e82f7a3..77b2aa2 100644 --- a/datadogriver/go.mod +++ b/datadogriver/go.mod @@ -1,13 +1,15 @@ module github.com/riverqueue/rivercontrib/datadogriver -go 1.22.0 +go 1.23.0 + +toolchain go1.24.2 require ( github.com/DataDog/dd-trace-go/v2 v2.0.0-rc.4 - github.com/riverqueue/river v0.19.0 - github.com/riverqueue/river/riverdriver/riverpgxv5 v0.19.0 - github.com/riverqueue/river/rivershared v0.19.0 - github.com/riverqueue/river/rivertype v0.19.0 + github.com/riverqueue/river v0.21.0 + github.com/riverqueue/river/riverdriver/riverpgxv5 v0.21.0 + github.com/riverqueue/river/rivershared v0.21.0 + github.com/riverqueue/river/rivertype v0.21.0 github.com/riverqueue/rivercontrib/otelriver v0.4.0 go.opentelemetry.io/otel v1.35.0 ) @@ -47,7 +49,7 @@ require ( github.com/hashicorp/go-sockaddr v1.0.5 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect - github.com/jackc/pgx/v5 v5.7.2 // indirect + github.com/jackc/pgx/v5 v5.7.4 // indirect github.com/jackc/puddle/v2 v2.2.2 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/lufia/plan9stats v0.0.0-20240226150601-1dcf7310316a // indirect @@ -61,7 +63,7 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/power-devops/perfstat v0.0.0-20240221224432-82ca36839d55 // indirect github.com/puzpuzpuz/xsync/v3 v3.5.1 // indirect - github.com/riverqueue/river/riverdriver v0.19.0 // indirect + github.com/riverqueue/river/riverdriver v0.21.0 // indirect github.com/ryanuber/go-glob v1.0.0 // indirect github.com/secure-systems-lab/go-securesystemslib v0.9.0 // indirect github.com/shirou/gopsutil/v4 v4.25.1 // indirect diff --git a/datadogriver/go.sum b/datadogriver/go.sum index 83cdcc4..84084ad 100644 --- a/datadogriver/go.sum +++ b/datadogriver/go.sum @@ -97,6 +97,8 @@ github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7Ulw github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= github.com/jackc/pgx/v5 v5.7.2 h1:mLoDLV6sonKlvjIEsV56SkWNCnuNv531l94GaIzO+XI= github.com/jackc/pgx/v5 v5.7.2/go.mod h1:ncY89UGWxg82EykZUwSpUKEfccBGGYq1xjrOpsbsfGQ= +github.com/jackc/pgx/v5 v5.7.4 h1:9wKznZrhWa2QiHL+NjTSPP6yjl3451BX3imWDnokYlg= +github.com/jackc/pgx/v5 v5.7.4/go.mod h1:ncY89UGWxg82EykZUwSpUKEfccBGGYq1xjrOpsbsfGQ= github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= github.com/json-iterator/go v1.1.12 h1:PV8peI4a0ysnczrg+LtxykD8LfKY9ML6u2jnxaEnrnM= @@ -141,18 +143,28 @@ github.com/richardartoul/molecule v1.0.1-0.20240531184615-7ca0df43c0b3 h1:4+LEVO github.com/richardartoul/molecule v1.0.1-0.20240531184615-7ca0df43c0b3/go.mod h1:vl5+MqJ1nBINuSsUI2mGgH79UweUT/B5Fy8857PqyyI= github.com/riverqueue/river v0.19.0 h1:WRh/NXhp+WEEY0HpCYgr4wSRllugYBt30HtyQ3jlz08= github.com/riverqueue/river v0.19.0/go.mod h1:YJ7LA2uBdqFHQJzKyYc+X6S04KJeiwsS1yU5a1rynlk= +github.com/riverqueue/river v0.21.0 h1:8bTEI664KIOFF18yFb2DX5ReyKb6YJ8MZIJArmbToNI= +github.com/riverqueue/river v0.21.0/go.mod h1:I5Fk3PvBl9i+fSjGLwLbVOWx7oTkrho5X6de67zQonA= github.com/riverqueue/river/riverdriver v0.19.0 h1:NyHz5DfB13paT2lvaO0CKmwy4SFLbA7n6MFRGRtwii4= github.com/riverqueue/river/riverdriver v0.19.0/go.mod h1:Soxi08hHkEvopExAp6ADG2437r4coSiB4QpuIL5E28k= +github.com/riverqueue/river/riverdriver v0.21.0 h1:DgluUSBiTcNOMNCJ3U7LHqAqYO5uQl1I5lupiXddYwY= +github.com/riverqueue/river/riverdriver v0.21.0/go.mod h1:Sa4sKc8e0InTH4MNS0foltRpkiiAvY5ZocknmLbI9/Q= github.com/riverqueue/river/riverdriver/riverdatabasesql v0.19.0 h1:ytdPnueiv7ANxJcntBtYenrYZZLY5P0mXoDV0l4WsLk= github.com/riverqueue/river/riverdriver/riverdatabasesql v0.19.0/go.mod h1:5Fahb3n+m1V0RAb0JlOIpzimoTlkOgudMfxSSCTcmFk= github.com/riverqueue/river/riverdriver/riverpgxv5 v0.19.0 h1:QWg7VTDDXbtTF6srr7Y1C888PiNzqv379yQuNSnH2hg= github.com/riverqueue/river/riverdriver/riverpgxv5 v0.19.0/go.mod h1:uvF1YS+iSQavCIHtaB/Y6O8A6Dnn38ctVQCpCpmHDZE= +github.com/riverqueue/river/riverdriver/riverpgxv5 v0.21.0 h1:k4hzqeH/ntaOmEheKdCvFbB7SU+e1emct/SG0n2nrPM= +github.com/riverqueue/river/riverdriver/riverpgxv5 v0.21.0/go.mod h1:dWTf+MUtkgfSmaCUaJmB//VE/uXHOOCJsEgor92nENw= github.com/riverqueue/river/rivershared v0.19.0 h1:TZvFM6CC+QgwQQUMQ5Ueuhx25ptgqcKqZQGsdLJnFeE= github.com/riverqueue/river/rivershared v0.19.0/go.mod h1:JAvmohuC5lounVk8e3zXZIs07Da3klzEeJo1qDQIbjw= +github.com/riverqueue/river/rivershared v0.21.0 h1:yRdCFfrVwVdi07m6NLjCdNePjwPt6wh7T8Fi1RbAncA= +github.com/riverqueue/river/rivershared v0.21.0/go.mod h1:eb61dWdwdXWaFa2fJ7BOiUe0hxe463f3g+oRDstw6/8= github.com/riverqueue/river/rivertype v0.19.0 h1:5rwgdh21pVcU9WjrHIIO9qC2dOMdRrrZ/HZZOE0JRyY= github.com/riverqueue/river/rivertype v0.19.0/go.mod h1:DETcejveWlq6bAb8tHkbgJqmXWVLiFhTiEm8j7co1bE= -github.com/riverqueue/rivercontrib/otelriver v0.2.0 h1:dYAwoh6+wNeSttiRmFS53nFRDGsXrN9e7GCMdhq5dsA= -github.com/riverqueue/rivercontrib/otelriver v0.2.0/go.mod h1:UvPxRkGbCfIgvVQ4sgYbYZaGS0UYmweaV/QD2L4B+rU= +github.com/riverqueue/river/rivertype v0.21.0 h1:4BhZ+T2pdVP3qqJYz9SJX4tR855B3UulRsjMUBXrywg= +github.com/riverqueue/river/rivertype v0.21.0/go.mod h1:lmdl3vLNDfchDWbYdW2uAocIuwIN+ZaXqAukdSCFqWs= +github.com/riverqueue/rivercontrib/otelriver v0.4.0 h1:UaiDCPc/rn+q6UECl5xfhAb/NhHzYU2vRjGyj2ZWAF4= +github.com/riverqueue/rivercontrib/otelriver v0.4.0/go.mod h1:PRm8MJ3BFKvpXfbaE2r1QgUb2RfhgwZF6YawAa4/c0s= github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= github.com/rogpeppe/go-internal v1.13.1 h1:KvO1DLK/DRN07sQ1LQKScxyZJuNnedQ5/wKSR38lUII= @@ -242,8 +254,8 @@ go.opentelemetry.io/otel/metric v1.35.0 h1:0znxYu2SNyuMSQT4Y9WDWej0VpcsxkuklLa4/ go.opentelemetry.io/otel/metric v1.35.0/go.mod h1:nKVFgxBZ2fReX6IlyW28MgZojkoAkJGaE8CpgeAU3oE= go.opentelemetry.io/otel/sdk v1.35.0 h1:iPctf8iprVySXSKJffSS79eOjl9pvxV9ZqOWT0QejKY= go.opentelemetry.io/otel/sdk v1.35.0/go.mod h1:+ga1bZliga3DxJ3CQGg3updiaAJoNECOgJREo9KHGQg= -go.opentelemetry.io/otel/sdk/metric v1.34.0 h1:5CeK9ujjbFVL5c1PhLuStg1wxA7vQv7ce1EK0Gyvahk= -go.opentelemetry.io/otel/sdk/metric v1.34.0/go.mod h1:jQ/r8Ze28zRKoNRdkjCZxfs6YvBTG1+YIqyFVFYec5w= +go.opentelemetry.io/otel/sdk/metric v1.35.0 h1:1RriWBmCKgkeHEhM7a2uMjMUfP7MsOF5JpUCaEqEI9o= +go.opentelemetry.io/otel/sdk/metric v1.35.0/go.mod h1:is6XYCUMpcKi+ZsOvfluY5YstFnhW0BidkR+gL+qN+w= go.opentelemetry.io/otel/trace v1.35.0 h1:dPpEfJu1sDIqruz7BHFG3c7528f6ddfSWfFDVt/xgMs= go.opentelemetry.io/otel/trace v1.35.0/go.mod h1:WUk7DtFp1Aw2MkvqGdwiXYDZZNvA/1J8o6xRXLrIkyc= go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc= diff --git a/go.work b/go.work index 577c6db..30ee857 100644 --- a/go.work +++ b/go.work @@ -1,6 +1,7 @@ -go 1.24.1 +go 1.24.2 use ( ./datadogriver ./otelriver + ./nilerror ) diff --git a/nilerror/README.md b/nilerror/README.md new file mode 100644 index 0000000..03f4cec --- /dev/null +++ b/nilerror/README.md @@ -0,0 +1,19 @@ +# nilerror [![Build Status](https://github.com/riverqueue/rivercontrib/actions/workflows/ci.yaml/badge.svg?branch=master)](https://github.com/riverqueue/rivercontrib/actions) [![Go Reference](https://pkg.go.dev/badge/github.com/riverqueue/rivercontrib.svg)](https://pkg.go.dev/github.com/riverqueue/rivercontrib/nilerror) + +Provides a River hook for detecting a common Go error where a nil struct value is wrapped in a non-nil interface value. This commonly causes trouble with the error interface, where an unintentional nil error is returned. + +See https://go.dev/doc/faq#nil_error. + +See [`example_hook_test.go`](./example_hook_test.go) for usage details. + +## Options + +The hook supports these options: + +``` go +hook := nilerror.NewHook(&HookConfig{ + Suppress: true, +}) +``` + +* `Suppress`: Causes the hook to suppress detected nil struct values wrapped in non-nil error interface values and produce warning logging instead. \ No newline at end of file diff --git a/nilerror/example_hook_test.go b/nilerror/example_hook_test.go new file mode 100644 index 0000000..2144546 --- /dev/null +++ b/nilerror/example_hook_test.go @@ -0,0 +1,31 @@ +package nilerror_test + +import ( + "log/slog" + + "github.com/rivercontrib/nilerror" + "github.com/riverqueue/river" + "github.com/riverqueue/river/riverdriver/riverpgxv5" + "github.com/riverqueue/river/rivershared/util/slogutil" + "github.com/riverqueue/river/rivertype" +) + +func ExampleHook() { + _, err := river.NewClient(riverpgxv5.New(nil), &river.Config{ + Hooks: []rivertype.Hook{ + // Install a nilerror check that will return an error when it + // detects a nil struct wrapped in a non-nil error interface. + nilerror.NewHook(nil), + + // Alternatively, suppress errors and produce warning logging. + nilerror.NewHook(&nilerror.HookConfig{Suppress: true}), + }, + Logger: slog.New(&slogutil.SlogMessageOnlyHandler{Level: slog.LevelWarn}), + TestOnly: true, // suitable only for use in tests; remove for live environments + }) + if err != nil { + panic(err) + } + + // Output: +} diff --git a/nilerror/go.mod b/nilerror/go.mod new file mode 100644 index 0000000..ef12469 --- /dev/null +++ b/nilerror/go.mod @@ -0,0 +1,30 @@ +module github.com/rivercontrib/nilerror + +go 1.24.2 + +require ( + github.com/riverqueue/river v0.21.0 + github.com/riverqueue/river/riverdriver/riverpgxv5 v0.21.0 + github.com/riverqueue/river/rivershared v0.21.0 + github.com/riverqueue/river/rivertype v0.21.0 + github.com/stretchr/testify v1.10.0 +) + +require ( + github.com/davecgh/go-spew v1.1.1 // indirect + github.com/jackc/pgpassfile v1.0.0 // indirect + github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 // indirect + github.com/jackc/pgx/v5 v5.7.4 // indirect + github.com/jackc/puddle/v2 v2.2.2 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/riverqueue/river/riverdriver v0.21.0 // indirect + github.com/tidwall/gjson v1.18.0 // indirect + github.com/tidwall/match v1.1.1 // indirect + github.com/tidwall/pretty v1.2.1 // indirect + github.com/tidwall/sjson v1.2.5 // indirect + go.uber.org/goleak v1.3.0 // indirect + golang.org/x/crypto v0.37.0 // indirect + golang.org/x/sync v0.13.0 // indirect + golang.org/x/text v0.24.0 // indirect + gopkg.in/yaml.v3 v3.0.1 // indirect +) diff --git a/nilerror/go.sum b/nilerror/go.sum new file mode 100644 index 0000000..beaafff --- /dev/null +++ b/nilerror/go.sum @@ -0,0 +1,66 @@ +github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= +github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/jackc/pgerrcode v0.0.0-20240316143900-6e2875d9b438 h1:Dj0L5fhJ9F82ZJyVOmBx6msDp/kfd1t9GRfny/mfJA0= +github.com/jackc/pgerrcode v0.0.0-20240316143900-6e2875d9b438/go.mod h1:a/s9Lp5W7n/DD0VrVoyJ00FbP2ytTPDVOivvn2bMlds= +github.com/jackc/pgpassfile v1.0.0 h1:/6Hmqy13Ss2zCq62VdNG8tM1wchn8zjSGOBJ6icpsIM= +github.com/jackc/pgpassfile v1.0.0/go.mod h1:CEx0iS5ambNFdcRtxPj5JhEz+xB6uRky5eyVu/W2HEg= +github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761 h1:iCEnooe7UlwOQYpKFhBabPMi4aNAfoODPEFNiAnClxo= +github.com/jackc/pgservicefile v0.0.0-20240606120523-5a60cdf6a761/go.mod h1:5TJZWKEWniPve33vlWYSoGYefn3gLQRzjfDlhSJ9ZKM= +github.com/jackc/pgx/v5 v5.7.4 h1:9wKznZrhWa2QiHL+NjTSPP6yjl3451BX3imWDnokYlg= +github.com/jackc/pgx/v5 v5.7.4/go.mod h1:ncY89UGWxg82EykZUwSpUKEfccBGGYq1xjrOpsbsfGQ= +github.com/jackc/puddle/v2 v2.2.2 h1:PR8nw+E/1w0GLuRFSmiioY6UooMp6KJv0/61nB7icHo= +github.com/jackc/puddle/v2 v2.2.2/go.mod h1:vriiEXHvEE654aYKXXjOvZM39qJ0q+azkZFrfEOc3H4= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= +github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw= +github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= +github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= +github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/riverqueue/river v0.21.0 h1:8bTEI664KIOFF18yFb2DX5ReyKb6YJ8MZIJArmbToNI= +github.com/riverqueue/river v0.21.0/go.mod h1:I5Fk3PvBl9i+fSjGLwLbVOWx7oTkrho5X6de67zQonA= +github.com/riverqueue/river/riverdriver v0.21.0 h1:DgluUSBiTcNOMNCJ3U7LHqAqYO5uQl1I5lupiXddYwY= +github.com/riverqueue/river/riverdriver v0.21.0/go.mod h1:Sa4sKc8e0InTH4MNS0foltRpkiiAvY5ZocknmLbI9/Q= +github.com/riverqueue/river/riverdriver/riverdatabasesql v0.21.0 h1:9hrg15avu0nPCD35OAFhOyEr6/MU+Xg66+Y5VY1PQtg= +github.com/riverqueue/river/riverdriver/riverdatabasesql v0.21.0/go.mod h1:vU1MlD7jzAeR0LsJ2kpGTLultgHi03IlEMAp3Uxz1bM= +github.com/riverqueue/river/riverdriver/riverpgxv5 v0.21.0 h1:k4hzqeH/ntaOmEheKdCvFbB7SU+e1emct/SG0n2nrPM= +github.com/riverqueue/river/riverdriver/riverpgxv5 v0.21.0/go.mod h1:dWTf+MUtkgfSmaCUaJmB//VE/uXHOOCJsEgor92nENw= +github.com/riverqueue/river/rivershared v0.21.0 h1:yRdCFfrVwVdi07m6NLjCdNePjwPt6wh7T8Fi1RbAncA= +github.com/riverqueue/river/rivershared v0.21.0/go.mod h1:eb61dWdwdXWaFa2fJ7BOiUe0hxe463f3g+oRDstw6/8= +github.com/riverqueue/river/rivertype v0.21.0 h1:4BhZ+T2pdVP3qqJYz9SJX4tR855B3UulRsjMUBXrywg= +github.com/riverqueue/river/rivertype v0.21.0/go.mod h1:lmdl3vLNDfchDWbYdW2uAocIuwIN+ZaXqAukdSCFqWs= +github.com/robfig/cron/v3 v3.0.1 h1:WdRxkvbJztn8LMz/QEvLN5sBU+xKpSqwwUO1Pjr4qDs= +github.com/robfig/cron/v3 v3.0.1/go.mod h1:eQICP3HwyT7UooqI/z+Ov+PtYAWygg1TEWWzGIFLtro= +github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8= +github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4= +github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= +github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= +github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= +github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/tidwall/gjson v1.14.2/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= +github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= +github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= +github.com/tidwall/match v1.1.1/go.mod h1:eRSPERbgtNPcGhD8UCthc6PmLEQXEWd3PRB5JTxsfmM= +github.com/tidwall/pretty v1.2.0/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= +github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/tidwall/sjson v1.2.5 h1:kLy8mja+1c9jlljvWTlSazM7cKDRfJuR/bOJhcY5NcY= +github.com/tidwall/sjson v1.2.5/go.mod h1:Fvgq9kS/6ociJEDnK0Fk1cpYF4FIW6ZF7LAe+6jwd28= +go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= +golang.org/x/crypto v0.37.0 h1:kJNSjF/Xp7kU0iB2Z+9viTPMW4EqqsrywMXLJOOsXSE= +golang.org/x/crypto v0.37.0/go.mod h1:vg+k43peMZ0pUMhYmVAWysMK35e6ioLh3wB8ZCAfbVc= +golang.org/x/sync v0.13.0 h1:AauUjRAJ9OSnvULf/ARrrVywoJDy0YS2AwQ98I37610= +golang.org/x/sync v0.13.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA= +golang.org/x/text v0.24.0 h1:dd5Bzh4yt5KYA8f9CJHCP4FB4D51c2c6JvN37xJJkJ0= +golang.org/x/text v0.24.0/go.mod h1:L8rBsPeo2pSS+xqN0d5u2ikmjtmoJbDBT1b7nHvFCdU= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= +gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/nilerror/hook.go b/nilerror/hook.go new file mode 100644 index 0000000..2971a45 --- /dev/null +++ b/nilerror/hook.go @@ -0,0 +1,76 @@ +// Package nilerror provides a River hook for detecting a common Go error where +// a nil struct value is wrapped in a non-nil interface value. This commonly +// causes trouble with the error interface, where an unintentional nil error is +// returned. +// +// See: https://go.dev/doc/faq#nil_error. +// +// The package must use reflection to work, and therefore necessitates some +// overhead from doing so. Its recommended use is to only the hook in test +// environments and detect non-nil nil errors via thorough testing, but it can +// only be used in production environments and configured to return an error on +// a detected problem or log a warning. +package nilerror + +import ( + "context" + "errors" + "fmt" + "reflect" + "strings" + + "github.com/riverqueue/river/rivershared/baseservice" + "github.com/riverqueue/river/rivertype" +) + +// HookConfig is configuration for the nilerror hook. +type HookConfig struct { + // Suppress causes the hook to suppress detected nil struct values wrapped + // in non-nil error interface values and produce warning logging instead. + Suppress bool +} + +// Hook is a River hook that detects nil error structs accidentally wrapped in a +// non-nil error interface, and either returns an error or logs a warning. +// +// See: https://go.dev/doc/faq#nil_error. +type Hook struct { + baseservice.BaseService + rivertype.Hook + config *HookConfig +} + +// NewHook initializes a new River nilerror hook. +// +// config may be nil. +func NewHook(config *HookConfig) *Hook { + if config == nil { + config = &HookConfig{} + } + return &Hook{config: config} +} + +func (h *Hook) WorkEnd(ctx context.Context, err error) error { + if err != nil { + errVal := reflect.ValueOf(err) + if errVal.IsNil() { + var ( + nonPtrType = errVal.Type().Elem() + packagePath = nonPtrType.PkgPath() + lastSlash = strings.LastIndex(packagePath, "/") + packageName = packagePath[lastSlash+1:] + nilPtrName = fmt.Sprintf("(*%s.%s)()", packageName, nonPtrType.Name()) + message = fmt.Sprintf("non-nil error containing nil internal value (see: https://go.dev/doc/faq#nil_error); probably a bug: %s", nilPtrName) + ) + + if h.config.Suppress { + h.Logger.WarnContext(ctx, h.Name+": Got "+message) + return nil + } + + return errors.New(message) + } + } + + return err +} diff --git a/nilerror/hook_test.go b/nilerror/hook_test.go new file mode 100644 index 0000000..698b1c3 --- /dev/null +++ b/nilerror/hook_test.go @@ -0,0 +1,92 @@ +package nilerror + +import ( + "bytes" + "context" + "log/slog" + "testing" + + "github.com/riverqueue/river/rivershared/baseservice" + "github.com/riverqueue/river/rivershared/riversharedtest" + "github.com/riverqueue/river/rivershared/util/slogutil" + "github.com/riverqueue/river/rivertype" + "github.com/stretchr/testify/require" +) + +// Verify interface compliance. +var ( + _ rivertype.HookWorkEnd = &Hook{} +) + +type myCustomError struct{} + +func (*myCustomError) Error() string { + return "my custom error" +} + +func TestHook(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + type testBundle struct{} + + setupConfig := func(t *testing.T, config *HookConfig) (*Hook, *testBundle) { + t.Helper() + + return baseservice.Init( + riversharedtest.BaseServiceArchetype(t), + NewHook(config), + ), &testBundle{} + } + + setup := func(t *testing.T) (*Hook, *testBundle) { + t.Helper() + + return setupConfig(t, nil) + } + + t.Run("NoError", func(t *testing.T) { + t.Parallel() + + hook, _ := setup(t) + + require.NoError(t, hook.WorkEnd(ctx, nil)) + }) + + t.Run("NonNilError", func(t *testing.T) { + t.Parallel() + + hook, _ := setup(t) + + myCustomErr := &myCustomError{} + require.Equal(t, myCustomErr, hook.WorkEnd(ctx, myCustomErr)) + }) + + t.Run("NilError", func(t *testing.T) { + t.Parallel() + + hook, _ := setup(t) + + var myCustomErr *myCustomError + require.EqualError(t, hook.WorkEnd(ctx, myCustomErr), + "non-nil error containing nil internal value (see: https://go.dev/doc/faq#nil_error); probably a bug: (*nilerror.myCustomError)()", + ) + }) + + t.Run("Suppress", func(t *testing.T) { + t.Parallel() + + hook, _ := setupConfig(t, &HookConfig{Suppress: true}) + + var logBuf bytes.Buffer + hook.Archetype.Logger = slog.New(&slogutil.SlogMessageOnlyHandler{Level: slog.LevelWarn, Out: &logBuf}) + + var myCustomErr *myCustomError + require.NoError(t, hook.WorkEnd(ctx, myCustomErr)) + + require.Equal(t, + "nilerror.Hook: Got non-nil error containing nil internal value (see: https://go.dev/doc/faq#nil_error); probably a bug: (*nilerror.myCustomError)()\n", + logBuf.String()) + }) +} diff --git a/otelriver/example_middleware_test.go b/otelriver/example_middleware_test.go index 83e5df1..5300cbe 100644 --- a/otelriver/example_middleware_test.go +++ b/otelriver/example_middleware_test.go @@ -10,7 +10,7 @@ import ( "github.com/riverqueue/rivercontrib/otelriver" ) -func Example_middleware() { +func ExampleMiddleware() { _, err := river.NewClient(riverpgxv5.New(nil), &river.Config{ Logger: slog.New(&slogutil.SlogMessageOnlyHandler{Level: slog.LevelWarn}), Middleware: []rivertype.Middleware{ diff --git a/otelriver/go.mod b/otelriver/go.mod index d578e55..58ed97d 100644 --- a/otelriver/go.mod +++ b/otelriver/go.mod @@ -1,6 +1,8 @@ module github.com/riverqueue/rivercontrib/otelriver -go 1.22.0 +go 1.23.0 + +toolchain go1.24.2 require ( github.com/riverqueue/river v0.19.0