Dependency updates and cleanup.

[slr71]

No description provided.

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	github.com/​cyverse-de/​configurate@​v0.0.0-20220113221928-13d34aae3f0f ⏵ v0.0.0-20260305004742-e3d1c1150f1e
	github.com/​lib/​pq@​v1.10.9 ⏵ v1.12.3
	go.opentelemetry.io/​contrib/​instrumentation/​github.com/​gorilla/​mux/​otelmux@​v0.49.0 ⏵ v0.68.0	-1
	github.com/​sirupsen/​logrus@​v1.9.3 ⏵ v1.9.4	+1
	github.com/​spf13/​viper@​v1.18.2 ⏵ v1.21.0
	github.com/​cyverse-de/​go-mod/​otelutils@​v0.0.3 ⏵ v0.0.6

View full report

[johnworth]

Code review

Found 1 issue:

1. The new logutil.LogIfError helper logs every non-nil error, but the codebase uses the defer tx.Rollback() idiom across handlers that subsequently call tx.Commit(). After a successful commit, the deferred rollback returns sql.ErrTxDone, which the helper now logs at error level — so every successful write will emit a spurious error log. The prior // nolint:errcheck silently dropped this.

Helper (logs any non-nil error, no filter for sql.ErrTxDone):

https://github.com/cyverse-de/async-tasks/blob/5902e4873e267b647b9285c493a88e0e29db88ce/internal/logutil/logutil.go#L11-L17

Representative call site — DeleteByIdRequest defers a rollback (L120) then commits (L139); on success the deferred rollback fires and logs sql.ErrTxDone:

async-tasks/app.go

Lines 119 to 140 in 5902e48

	}
	defer logutil.LogIfError(log, tx.Rollback)
	task, err := tx.GetTask(ctx, id, true)
	if err != nil {
	errored(writer, err.Error())
	return
	}
	if task.ID == "" {
	notFound(writer, "not found")
	return
	}
	err = tx.DeleteTask(ctx, id)
	if err != nil {
	errored(writer, err.Error())
	return
	}
	err = tx.Commit()
	if err != nil {

Same pattern affects CreateTaskRequest, AddStatusRequest, AddBehaviorRequest in app.go, finishTask/deleteTask in updater.go, and processSingleTask. In Processor there is an additional explicit rollback after GetTasksByFilter followed by the deferred one, so even the read-only path logs sql.ErrTxDone on every tick:

async-tasks/behaviors/statuschangetimeout/statuschangetimeout.go

Lines 135 to 144 in 5902e48

	}
	defer logutil.LogIfError(log, tx.Rollback)
	tasks, err := tx.GetTasksByFilter(ctx, filter, "end_date IS NOT NULL DESC")
	if err != nil {
	return err
	}
	logutil.LogIfError(log, tx.Rollback)

Suggested fix: have LogIfError (or a dedicated RollbackUnlessCommitted-style helper) skip sql.ErrTxDone, or track a committed flag and only roll back when the commit didn't happen.

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[johnworth]

Re-review of 23567e9

The fix resolves the spurious-error-log issue I flagged. errorsToIgnore containing sql.ErrTxDone covers all the call sites I noted: the defer ... tx.Rollback + tx.Commit pattern in app.go/updater.go, and the explicit-then-deferred rollback pair in Processor. database/sql.Tx.Rollback() returns sql.ErrTxDone as a bare sentinel (no wrapping), so the map's value-equality lookup matches correctly for every affected path. Tests pass locally and the table-driven test auto-extends to any future entries in errorsToIgnore.

Optional nit, not a blocker: errors.Is(err, sql.ErrTxDone) is the more idiomatic check and would still match if anything ever wrapped the sentinel — switching to a slices/for-loop with errors.Is would future-proof it. The current map works for today's call sites either way.

LGTM.

🤖 Generated with Claude Code

[slr71]

Good catch. I found myself wondering if I should make the errors to skip customizable for different cases, so I'll do that now.

[johnworth]

LGTM

[slr71]

Thanks for the reviews. 👍