Don't error on cancelled startup to make it not necessary to export ErrShutdown (#841)

This change's driven by the proposal in #830. When client start up is
cancelled by a stop, we'd return `ErrShutdown` as a special signal. The
problem is that `ErrShutdown` was never exported, so this wasn't really
usable by outside callers.

In #830 it was proposed to export `ErrShutdown`. However, after some
discussion, we suspect that it might be better to leave it unexported,
and then have `Start` return no error on cancelled startup. This
minimizes the number of startup conditions callers have to handle.

Because `ErrShutdown` wasn't previously exported, we should be able to
make this change safely without breaking anyone.

@mastercactapus did bring up a fair point that the alternative of
exporting `ErrShutdown` would allow a caller to definitively tell the
difference between the client successfully starting and backgrounding
itself versus one whose start was cancelled. I think we're okay not to
require that differentiation though because if `ErrShutdown` would've be
returned, then the caller would have called `Stop`, so presumably they'd
know to expect an successful start.

I figure that in case a strong case for wanting to differentiate the
cases comes through, we could always resurrect the error.

Also, rename the internal `ErrShutdown` to `ErrStop` to be more
consistent with the naming of `Client.Stop` (which had previously been
called `Shutdown`). This won't make any difference, but will be a little
better in case we ever do export it and then forget to rename it at that
time.

Author: brandur

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Client no longer returns an error if stopped before startup could complete (previously, it returned the unexported `ErrShutdown`). [PR #841](https://github.com/riverqueue/river/pull/841).
+
 ## [0.20.2] - 2025-04-08
 
 ### Added

client.go

@@ -888,7 +888,7 @@ func (c *Client[TTx]) Start(ctx context.Context) error {
 	}(); err != nil {
 		defer stopped()
 		if errors.Is(context.Cause(fetchCtx), startstop.ErrStop) {
-			return rivercommon.ErrShutdown
+			return nil
 		}
 		return err
 	}
@@ -928,7 +928,7 @@ func (c *Client[TTx]) Start(ctx context.Context) error {
 		startstop.StopAllParallel(producersAsServices()...)
 		c.baseService.Logger.DebugContext(ctx, c.baseService.Name+": All producers stopped")
 
-		c.workCancel(rivercommon.ErrShutdown)
+		c.workCancel(rivercommon.ErrStop)
 
 		// Stop all mainline services where stop order isn't important.
 		startstop.StopAllParallel(append(
@@ -985,7 +985,7 @@ func (c *Client[TTx]) Stop(ctx context.Context) error {
 // instead.
 func (c *Client[TTx]) StopAndCancel(ctx context.Context) error {
 	c.baseService.Logger.InfoContext(ctx, c.baseService.Name+": Hard stop started; cancelling all work")
-	c.workCancel(rivercommon.ErrShutdown)
+	c.workCancel(rivercommon.ErrStop)
 
 	shouldStop, stopped, finalizeStop := c.baseStartStop.StopInit()
 	if !shouldStop {

client_test.go

@@ -344,10 +344,7 @@ func Test_Client(t *testing.T) {
 					err := client.Queues().Add(fmt.Sprintf("new_queue_%d_%d_before", workerNum, j), QueueConfig{MaxWorkers: 1})
 					require.NoError(t, err)
 
-					err = client.Start(ctx)
-					if !errors.Is(err, rivercommon.ErrShutdown) {
-						require.NoError(t, err)
-					}
+					require.NoError(t, client.Start(ctx))
 
 					err = client.Queues().Add(fmt.Sprintf("new_queue_%d_%d_after", workerNum, j), QueueConfig{MaxWorkers: 1})
 					require.NoError(t, err)
@@ -1104,7 +1101,7 @@ func Test_Client(t *testing.T) {
 
 		clientWithStop := &clientWithSimpleStop[pgx.Tx]{Client: client}
 
-		startstoptest.StressErr(ctx, t, clientWithStop, rivercommon.ErrShutdown)
+		startstoptest.Stress(ctx, t, clientWithStop)
 	})
 }
 
@@ -1405,7 +1402,6 @@ func Test_Client_StopAndCancel(t *testing.T) {
 			t.Logf("Job waiting for context cancellation")
 			defer t.Logf("Job finished")
 			<-ctx.Done()
-			require.ErrorIs(t, context.Cause(ctx), rivercommon.ErrShutdown)
 			t.Logf("Job context done, closing chan and returning")
 			close(jobDoneChan)
 			return nil

internal/rivercommon/river_common.go

@@ -18,8 +18,7 @@ const (
 
 type ContextKeyClient struct{}
 
-// ErrShutdown is a special error injected by the client into its fetch and work
+// ErrStop is a special error injected by the client into its fetch and work
 // CancelCauseFuncs when it's stopping. It may be used by components for such
-// cases like avoiding logging an error during a normal shutdown procedure. This
-// is internal for the time being, but we could also consider exposing it.
-var ErrShutdown = errors.New("shutdown initiated")
+// cases like avoiding logging an error during a normal shutdown procedure.
+var ErrStop = errors.New("stop initiated")