Skip to content

Commit

Permalink
Improve error when Client.Subscribe called on a client that will no…
Browse files Browse the repository at this point in the history
…t work (#599)

Related to #596. If `Subscribe` was called on a client that didn't have
a `Workers` bundle configure a nil pointer panic would occur because
`subscriptionManager` was never initialized.

Here, leave that as a panic since it makes sense to warn a user about an
API misuse that'd undoubtedly lead to more confusion/pain, but improve
the error message so that it's more obvious to the caller why this is a
problem.

Fixes #596.
  • Loading branch information
brandur authored Sep 18, 2024
1 parent 64ca485 commit b4b8cfe
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Fixed a panic that'd occur if `StopAndCancel` was invoked before a client was started. [PR #557](https://github.com/riverqueue/river/pull/557).
- A `PeriodicJobConstructor` should be able to return `nil` `JobArgs` if it wishes to not have any job inserted. However, this was either never working or was broken at some point. It's now fixed. Thanks [@semanser](https://github.com/semanser)! [PR #572](https://github.com/riverqueue/river/pull/572).
- Fixed a nil pointer exception if `Client.Subscribe` was called when the client had no configured workers (it still, panics with a more instructive error message now). [PR #599](https://github.com/riverqueue/river/pull/599).

## [0.11.4] - 2024-08-20

Expand Down
4 changes: 4 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,10 @@ type SubscribeConfig struct {

// Special internal variant that lets us inject an overridden size.
func (c *Client[TTx]) SubscribeConfig(config *SubscribeConfig) (<-chan *Event, func()) {
if c.subscriptionManager == nil {
panic("created a subscription on a client that will never work jobs (Workers not configured)")
}

return c.subscriptionManager.SubscribeConfig(config)
}

Expand Down
13 changes: 13 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3893,6 +3893,19 @@ func Test_Client_Subscribe(t *testing.T) {

require.Empty(t, client.subscriptionManager.subscriptions)
})

// Just make sure this doesn't fail on a nil pointer exception.
t.Run("SubscribeOnClientWithoutWorkers", func(t *testing.T) {
t.Parallel()

dbPool := riverinternaltest.TestDB(ctx, t)

client := newTestClient(t, dbPool, &Config{})

require.PanicsWithValue(t, "created a subscription on a client that will never work jobs (Workers not configured)", func() {
_, _ = client.Subscribe(EventKindJobCompleted)
})
})
}

// SubscribeConfig uses all the same code as Subscribe, so these are just a
Expand Down

0 comments on commit b4b8cfe

Please sign in to comment.