diff --git a/coderd/webpush.go b/coderd/webpush.go index 893401552d..3e84aa012e 100644 --- a/coderd/webpush.go +++ b/coderd/webpush.go @@ -88,12 +88,15 @@ func (api *API) deleteUserWebpushSubscription(rw http.ResponseWriter, r *http.Re } // Return NotFound if the subscription does not exist. - if existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID); err != nil && errors.Is(err, sql.ErrNoRows) { - httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ - Message: "Webpush subscription not found.", + existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to get webpush subscriptions.", + Detail: err.Error(), }) return - } else if idx := slices.IndexFunc(existing, func(s database.WebpushSubscription) bool { + } + if idx := slices.IndexFunc(existing, func(s database.WebpushSubscription) bool { return s.Endpoint == req.Endpoint }); idx == -1 { httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ diff --git a/coderd/webpush_test.go b/coderd/webpush_test.go index f41639b99e..467f507881 100644 --- a/coderd/webpush_test.go +++ b/coderd/webpush_test.go @@ -1,13 +1,19 @@ package coderd_test import ( + "context" "net/http" "net/http/httptest" + "sync/atomic" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbtestutil" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -80,3 +86,54 @@ func TestWebpushSubscribeUnsubscribe(t *testing.T) { }) require.Error(t, err, "delete webpush subscription for another user") } + +// testWebpushErrorStore wraps a real database.Store and allows injecting +// errors into GetWebpushSubscriptionsByUserID. +type testWebpushErrorStore struct { + database.Store + getWebpushSubscriptionsErr atomic.Pointer[error] +} + +func (s *testWebpushErrorStore) GetWebpushSubscriptionsByUserID(ctx context.Context, userID uuid.UUID) ([]database.WebpushSubscription, error) { + if err := s.getWebpushSubscriptionsErr.Load(); err != nil { + return nil, *err + } + return s.Store.GetWebpushSubscriptionsByUserID(ctx, userID) +} + +func TestDeleteWebpushSubscription(t *testing.T) { + t.Parallel() + + t.Run("database error returns 500", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + + store, ps := dbtestutil.NewDB(t) + wrappedStore := &testWebpushErrorStore{Store: store} + + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentWebPush)} + client := coderdtest.New(t, &coderdtest.Options{ + DeploymentValues: dv, + Database: wrappedStore, + Pubsub: ps, + }) + owner := coderdtest.CreateFirstUser(t, client) + memberClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + // Inject a database error into + // GetWebpushSubscriptionsByUserID. The handler should + // return 500, not mask the error as 404. + dbErr := xerrors.New("database is unavailable") + wrappedStore.getWebpushSubscriptionsErr.Store(&dbErr) + + err := memberClient.DeleteWebpushSubscription(ctx, "me", codersdk.DeleteWebpushSubscription{ + Endpoint: "https://push.example.com/test", + }) + var sdkError *codersdk.Error + require.Error(t, err) + require.ErrorAsf(t, err, &sdkError, "error should be of type *codersdk.Error") + require.Equal(t, http.StatusInternalServerError, sdkError.StatusCode(), "database errors should return 500, not be masked as 404") + }) +}