From b4b8cfe4523f5b40336ec212a5c9ff4fcfcca957 Mon Sep 17 00:00:00 2001
From: Brandur Leach <brandur@brandur.org>
Date: Wed, 18 Sep 2024 08:04:51 -0700
Subject: [PATCH] Improve error when `Client.Subscribe` called on a client that
 will not 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.
---
 CHANGELOG.md   |  1 +
 client.go      |  4 ++++
 client_test.go | 13 +++++++++++++
 3 files changed, 18 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 45609fdf..2ef0e0dc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -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
 
diff --git a/client.go b/client.go
index a7dd4d23..05f35e75 100644
--- a/client.go
+++ b/client.go
@@ -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)
 }
 
diff --git a/client_test.go b/client_test.go
index e38cbd24..fd358893 100644
--- a/client_test.go
+++ b/client_test.go
@@ -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