bretton.dev
A community based topic aggregation platform built on atproto
fix(blocking): address PR review feedback - improve error handling
- Fix P1 issue: properly bubble up database errors instead of masking as conflict
* Only return ErrBlockAlreadyExists when getErr is ErrBlockNotFound (race condition)
* Real DB errors (outages, connection failures) now propagate to operators
- Remove unused V1 functions flagged by linter:
* createRecordOnPDS, deleteRecordOnPDS, callPDS (replaced by *As versions)
- Apply automatic code formatting via golangci-lint --fix:
* Align struct field tags in CommunityBlock
* Fix comment alignment across test files
* Remove trailing whitespace
- All tests passing, linter clean
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
+1
-1
internal/api/handlers/community/subscribe.go
+1
-1
internal/api/handlers/community/subscribe.go
···
34
34
35
35
// Parse request body
36
36
var req struct {
37
-
Community string `json:"community"` // DID only (per lexicon)
37
+
Community string `json:"community"` // DID only (per lexicon)
38
38
ContentVisibility int `json:"contentVisibility"` // Optional: 1-5 scale, defaults to 3
39
39
}
40
40
+2
-2
internal/core/communities/community.go
+2
-2
internal/core/communities/community.go
···
55
55
// CommunityBlock represents a user blocking a community
56
56
// Block records live in the user's repository (at://user_did/social.coves.community.block/{rkey})
57
57
type CommunityBlock struct {
58
-
ID int `json:"id" db:"id"`
58
+
BlockedAt time.Time `json:"blockedAt" db:"blocked_at"`
59
59
UserDID string `json:"userDid" db:"user_did"`
60
60
CommunityDID string `json:"communityDid" db:"community_did"`
61
-
BlockedAt time.Time `json:"blockedAt" db:"blocked_at"`
62
61
RecordURI string `json:"recordUri,omitempty" db:"record_uri"`
63
62
RecordCID string `json:"recordCid,omitempty" db:"record_cid"`
63
+
ID int `json:"id" db:"id"`
64
64
}
65
65
66
66
// Membership represents active participation with reputation tracking
+12
-40
internal/core/communities/service.go
+12
-40
internal/core/communities/service.go
···
5
5
"bytes"
6
6
"context"
7
7
"encoding/json"
8
+
"errors"
8
9
"fmt"
9
10
"io"
10
11
"log"
···
565
566
// Block exists in our index - return it
566
567
return existingBlock, nil
567
568
}
568
-
// Race condition: PDS has the block but Jetstream hasn't indexed it yet
569
-
// Return typed conflict error so handler can return 409 instead of 500
570
-
// This is normal in eventually-consistent systems
571
-
return nil, ErrBlockAlreadyExists
569
+
// Only treat as "already exists" if the error is ErrBlockNotFound (race condition)
570
+
// Any other error (DB outage, connection failure, etc.) should bubble up
571
+
if errors.Is(getErr, ErrBlockNotFound) {
572
+
// Race condition: PDS has the block but Jetstream hasn't indexed it yet
573
+
// Return typed conflict error so handler can return 409 instead of 500
574
+
// This is normal in eventually-consistent systems
575
+
return nil, ErrBlockAlreadyExists
576
+
}
577
+
// Real datastore error - bubble it up so operators see the failure
578
+
return nil, fmt.Errorf("PDS reported duplicate block but failed to fetch from index: %w", getErr)
572
579
}
573
580
return nil, fmt.Errorf("failed to create block on PDS: %w", err)
574
581
}
···
724
731
725
732
// PDS write-forward helpers
726
733
727
-
func (s *communityService) createRecordOnPDS(ctx context.Context, repoDID, collection, rkey string, record map[string]interface{}) (string, string, error) {
728
-
endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.createRecord", strings.TrimSuffix(s.pdsURL, "/"))
729
-
730
-
payload := map[string]interface{}{
731
-
"repo": repoDID,
732
-
"collection": collection,
733
-
"record": record,
734
-
}
735
-
736
-
if rkey != "" {
737
-
payload["rkey"] = rkey
738
-
}
739
-
740
-
return s.callPDS(ctx, "POST", endpoint, payload)
741
-
}
742
-
743
734
// createRecordOnPDSAs creates a record with a specific access token (for V2 community auth)
744
735
func (s *communityService) createRecordOnPDSAs(ctx context.Context, repoDID, collection, rkey string, record map[string]interface{}, accessToken string) (string, string, error) {
745
736
endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.createRecord", strings.TrimSuffix(s.pdsURL, "/"))
···
771
762
return s.callPDSWithAuth(ctx, "POST", endpoint, payload, accessToken)
772
763
}
773
764
774
-
func (s *communityService) deleteRecordOnPDS(ctx context.Context, repoDID, collection, rkey string) error {
775
-
endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.deleteRecord", strings.TrimSuffix(s.pdsURL, "/"))
776
-
777
-
payload := map[string]interface{}{
778
-
"repo": repoDID,
779
-
"collection": collection,
780
-
"rkey": rkey,
781
-
}
782
-
783
-
_, _, err := s.callPDS(ctx, "POST", endpoint, payload)
784
-
return err
785
-
}
786
-
787
765
// deleteRecordOnPDSAs deletes a record with a specific access token (for user-scoped deletions)
788
-
func (s *communityService) deleteRecordOnPDSAs(ctx context.Context, repoDID, collection, rkey string, accessToken string) error {
766
+
func (s *communityService) deleteRecordOnPDSAs(ctx context.Context, repoDID, collection, rkey, accessToken string) error {
789
767
endpoint := fmt.Sprintf("%s/xrpc/com.atproto.repo.deleteRecord", strings.TrimSuffix(s.pdsURL, "/"))
790
768
791
769
payload := map[string]interface{}{
···
796
774
797
775
_, _, err := s.callPDSWithAuth(ctx, "POST", endpoint, payload, accessToken)
798
776
return err
799
-
}
800
-
801
-
func (s *communityService) callPDS(ctx context.Context, method, endpoint string, payload map[string]interface{}) (string, string, error) {
802
-
// Use instance's access token
803
-
return s.callPDSWithAuth(ctx, method, endpoint, payload, s.pdsAccessToken)
804
777
}
805
778
806
779
// callPDSWithAuth makes a PDS call with a specific access token (V2: for community authentication)
···
870
843
}
871
844
872
845
// Helper functions
873
-
-1
internal/db/postgres/community_repo_blocks.go
-1
internal/db/postgres/community_repo_blocks.go
+6
-6
tests/integration/community_blocking_test.go
+6
-6
tests/integration/community_blocking_test.go
···
431
431
432
432
func createBlockingTestCommunity(t *testing.T, repo communities.Repository, name, did string) *communities.Community {
433
433
community := &communities.Community{
434
-
DID: did,
435
-
Handle: fmt.Sprintf("!%s@coves.test", name),
436
-
Name: name,
437
-
DisplayName: fmt.Sprintf("Test Community %s", name),
438
-
Description: "Test community for blocking tests",
439
-
OwnerDID: did,
434
+
DID: did,
435
+
Handle: fmt.Sprintf("!%s@coves.test", name),
436
+
Name: name,
437
+
DisplayName: fmt.Sprintf("Test Community %s", name),
438
+
Description: "Test community for blocking tests",
439
+
OwnerDID: did,
440
440
CreatedByDID: "did:plc:test-creator",
441
441
HostedByDID: "did:plc:test-instance",
442
442
Visibility: "public",
+4
-5
tests/integration/community_e2e_test.go
+4
-5
tests/integration/community_e2e_test.go
···
687
687
CID: subscribeResp.CID,
688
688
Record: map[string]interface{}{
689
689
"$type": "social.coves.community.subscription",
690
-
"subject": community.DID,
690
+
"subject": community.DID,
691
691
"contentVisibility": float64(5), // JSON numbers are float64
692
692
"createdAt": time.Now().Format(time.RFC3339),
693
693
},
···
771
771
CID: subscription.RecordCID,
772
772
Record: map[string]interface{}{
773
773
"$type": "social.coves.community.subscription",
774
-
"subject": community.DID,
774
+
"subject": community.DID,
775
775
"contentVisibility": float64(3),
776
776
"createdAt": time.Now().Format(time.RFC3339),
777
777
},
···
893
893
Operation: "delete",
894
894
Collection: "social.coves.community.subscription",
895
895
RKey: rkey,
896
-
CID: "", // No CID on deletes
897
-
Record: nil, // No record data on deletes
896
+
CID: "", // No CID on deletes
897
+
Record: nil, // No record data on deletes
898
898
},
899
899
}
900
900
if handleErr := consumer.HandleEvent(context.Background(), &deleteEvent); handleErr != nil {
···
1504
1504
1505
1505
return community
1506
1506
}
1507
-
1508
1507
1509
1508
// authenticateWithPDS authenticates with the PDS and returns access token and DID
1510
1509
func authenticateWithPDS(pdsURL, handle, password string) (string, string, error) {
+12
-12
tests/integration/subscription_indexing_test.go
+12
-12
tests/integration/subscription_indexing_test.go
···
46
46
RKey: rkey,
47
47
CID: "bafytest123",
48
48
Record: map[string]interface{}{
49
-
"$type": "social.coves.community.subscription",
50
-
"subject": community.DID,
49
+
"$type": "social.coves.community.subscription",
50
+
"subject": community.DID,
51
51
"createdAt": time.Now().Format(time.RFC3339),
52
52
"contentVisibility": float64(5), // JSON numbers decode as float64
53
53
},
···
101
101
RKey: rkey,
102
102
CID: "bafydefault",
103
103
Record: map[string]interface{}{
104
-
"$type": "social.coves.community.subscription",
105
-
"subject": community.DID,
104
+
"$type": "social.coves.community.subscription",
105
+
"subject": community.DID,
106
106
"createdAt": time.Now().Format(time.RFC3339),
107
107
// contentVisibility NOT provided
108
108
},
···
130
130
131
131
t.Run("clamps contentVisibility to valid range (1-5)", func(t *testing.T) {
132
132
testCases := []struct {
133
+
name string
133
134
input float64
134
135
expected int
135
-
name string
136
136
}{
137
137
{input: 0, expected: 1, name: "zero clamped to 1"},
138
138
{input: -5, expected: 1, name: "negative clamped to 1"},
···
201
201
RKey: rkey,
202
202
CID: "bafyidempotent",
203
203
Record: map[string]interface{}{
204
-
"$type": "social.coves.community.subscription",
205
-
"subject": community.DID,
204
+
"$type": "social.coves.community.subscription",
205
+
"subject": community.DID,
206
206
"createdAt": time.Now().Format(time.RFC3339),
207
207
"contentVisibility": float64(4),
208
208
},
···
268
268
RKey: rkey,
269
269
CID: "bafycreate",
270
270
Record: map[string]interface{}{
271
-
"$type": "social.coves.community.subscription",
272
-
"subject": community.DID,
271
+
"$type": "social.coves.community.subscription",
272
+
"subject": community.DID,
273
273
"createdAt": time.Now().Format(time.RFC3339),
274
274
"contentVisibility": float64(3),
275
275
},
···
298
298
Operation: "delete",
299
299
Collection: "social.coves.community.subscription",
300
300
RKey: rkey,
301
-
CID: "", // No CID on deletes
301
+
CID: "", // No CID on deletes
302
302
Record: nil, // No record data on deletes
303
303
},
304
304
}
···
390
390
RKey: rkey,
391
391
CID: "bafycount",
392
392
Record: map[string]interface{}{
393
-
"$type": "social.coves.community.subscription",
394
-
"subject": community.DID,
393
+
"$type": "social.coves.community.subscription",
394
+
"subject": community.DID,
395
395
"createdAt": time.Now().Format(time.RFC3339),
396
396
"contentVisibility": float64(3),
397
397
},