BCM-3707: add structured CONFLICT error codes - #175
Draft
iljapavlovs wants to merge 3 commits into
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
Adds first-class support in the SDK for structured HTTP 409 (CONFLICT) responses by mapping ApplicationError.code values to stable, shared sentinel errors (so errors.Is works consistently across packages).
Changes:
- Introduces canonical 409 conflict sentinels plus helpers to map/wrap conflict
ApplicationErrorpayloads (apierror.Conflict,apierror.WrapConflict,apierror.ConflictCode). - Updates multiple domain clients (channels, wallets, watchers, transact, queries) to explicitly handle HTTP 409 and return typed sentinel errors when a recognized conflict code is present.
- Adds/updates tests to validate conflict-code mapping behavior in
apierror,channels,queries, andtransact.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| apierror/apierror.go | Adds canonical conflict sentinels + mapping/wrapping helpers for HTTP 409 responses. |
| apierror/apierror_test.go | Adds unit tests for conflict mapping, wrapping, and code extraction. |
| channels/channels.go | Handles 409 for create/update and exposes ErrChannelAlreadyExists. |
| channels/channels_test.go | Updates create-conflict tests to use structured conflict payloads; adds missing-code fallback test. |
| wallets/wallets.go | Handles 409 for create/update; exposes wallet-related conflict sentinels. |
| watchers/watchers.go | Handles 409 for create/update/archive; exposes watcher conflict sentinels. |
| transact/transact.go | Handles 409 for create operation and draft finalize/cancel paths with conflict-code awareness. |
| transact/transact_test.go | Adds tests asserting conflict-code mapping is surfaced via errors.Is. |
| queries/queries.go | Adds ErrIdempotencyKeyMismatch and maps 409 to typed conflict sentinel when present. |
| queries/queries_test.go | Adds coverage for idempotency-key-mismatch conflict mapping. |
| go.mod | Bumps crec-api-go dependency to a newer pseudo-version that includes conflict codes. |
| go.sum | Updates checksums for the new crec-api-go version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
761
to
766
| case http.StatusConflict: | ||
| if mapped := apierror.Conflict(resp.JSON409); mapped != nil { | ||
| return nil, fmt.Errorf("%w: %w", ErrDraftNotFinalizable, mapped) | ||
| } | ||
| return nil, ErrDraftNotFinalizable | ||
| case http.StatusUnauthorized: |
Comment on lines
830
to
834
| case http.StatusConflict: | ||
| if mapped := apierror.Conflict(resp.JSON409); mapped != nil { | ||
| return fmt.Errorf("%w: %w", ErrDraftNotCancellable, mapped) | ||
| } | ||
| return ErrDraftNotCancellable |
Comment on lines
+270
to
+274
| case http.StatusConflict: | ||
| c.logger.Warn("Conflict when creating operation", | ||
| "channel_id", channelID.String(), | ||
| "code", apierror.ConflictCode(resp.JSON409)) | ||
| return nil, apierror.WrapConflict(resp.JSON409, ErrCreateOperation, "") |
Comment on lines
+278
to
+282
| case http.StatusConflict: | ||
| c.logger.Warn("Conflict when creating watcher with service", | ||
| "channel_id", channelID.String(), | ||
| "code", apierror.ConflictCode(resp.JSON409)) | ||
| return nil, apierror.WrapConflict(resp.JSON409, ErrCreateWatcherService, "channel ID "+channelID.String()) |
Comment on lines
+265
to
+269
| case http.StatusConflict: | ||
| c.logger.Warn("Conflict when creating wallet", | ||
| "name", input.Name, | ||
| "code", apierror.ConflictCode(resp.JSON409)) | ||
| return nil, apierror.WrapConflict(resp.JSON409, ErrCreateWallet, "name "+input.Name) |
Comment on lines
+324
to
+328
| case http.StatusConflict: | ||
| c.logger.Warn("Conflict when updating channel", | ||
| "channel_id", channelID.String(), | ||
| "code", apierror.ConflictCode(resp.JSON409)) | ||
| return nil, apierror.WrapConflict(resp.JSON409, ErrUpdateChannel, "channel ID "+channelID.String()) |
urielkelman
reviewed
Jul 16, 2026
| ErrOperationDeadlineElapsed = apierror.ErrOperationDeadlineElapsed | ||
| // ErrResourceVersionConflict is returned when an operation was modified concurrently by | ||
| // another request (409 response). | ||
| ErrResourceVersionConflict = apierror.ErrResourceVersionConflict |
Contributor
There was a problem hiding this comment.
i am thinking on this... i fell that we might be exposing an implementation detail (versions) of our entities on our api, which we shouldn't do imho.
instead we could say something more generic, like conflict because of concurrent update, without talking about "versions". wdty?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request overview
Adds first-class support in the SDK for structured HTTP 409 (CONFLICT) responses by mapping
ApplicationError.codevalues to stable, shared sentinel errors (soerrors.Isworks consistently across packages).Changes:
ApplicationErrorpayloads (apierror.Conflict,apierror.WrapConflict,apierror.ConflictCode).apierror,channels,queries, andtransact.