mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(coderd): fix error handling in deleteUserWebpushSubscription (#22500)
## Summary
`deleteUserWebpushSubscription` in `coderd/webpush.go` had incorrect
error handling that masked database errors as 404 responses.
## Bug
`GetWebpushSubscriptionsByUserID` is a `:many` query — it returns `([],
nil)` when no rows match, never `sql.ErrNoRows`. The previous `if/else
if` chain:
```go
if existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID); err != nil && errors.Is(err, sql.ErrNoRows) {
// dead code — :many queries never return sql.ErrNoRows
} else if idx := slices.IndexFunc(existing, ...); idx == -1 {
// real DB errors fall through here, existing is nil, idx is -1 → 404
}
```
Any real database error (connection failure, timeout, authorization
error) fell through to the `else if` branch where `slices.IndexFunc(nil,
...)` returns `-1`, returning 404 "subscription not found" instead of
500.
## Fix
Split into two separate checks so database errors properly return 500:
```go
existing, err := api.Database.GetWebpushSubscriptionsByUserID(ctx, user.ID)
if err != nil {
// 500
}
if idx := slices.IndexFunc(existing, ...); idx == -1 {
// 404
}
```
## Testing
Added `TestDeleteWebpushSubscription/database_error_returns_500` which
wraps the DB store to inject an error into
`GetWebpushSubscriptionsByUserID` and asserts the handler returns 500
(not 404).
This commit is contained in:
+7
-4
@@ -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{
|
||||
|
||||
Reference in New Issue
Block a user