fix: return 400 instead of 500 for idempotency key reuse with different body (#921)

flemzord · web-flow · commit 378b51d1d7d1 · 2025-05-16T10:54:51.000Z
diff --git a/.gitignore b/.gitignore
@@ -9,3 +9,4 @@ dumps
 .env
 !.envrc
 Pulumi.*.yaml
+coverage.txt
diff --git a/docs/api/README.md b/docs/api/README.md
@@ -966,6 +966,7 @@ Authorization ( Scopes: ledger:write )
 DELETE http://localhost:8080/v2/{ledger}/transactions/{id}/metadata/{key} HTTP/1.1
 Host: localhost:8080
 Accept: application/json
+Idempotency-Key: string
 
 ```
 
@@ -980,6 +981,7 @@ Delete metadata by key
 |ledger|path|string|true|Name of the ledger.|
 |id|path|integer(bigint)|true|Transaction ID.|
 |key|path|string|true|The key to remove.|
+|Idempotency-Key|header|string|false|Use an idempotency key|
 
 > Example responses
 
diff --git a/internal/api/common/errors.go b/internal/api/common/errors.go
@@ -2,11 +2,13 @@ package common
 
 import (
 	"errors"
+	"net/http"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/logging"
 	"github.com/formancehq/go-libs/v3/otlp"
 	"github.com/formancehq/go-libs/v3/platform/postgres"
-	"net/http"
+	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 )
 
 const (
@@ -37,6 +39,19 @@ func HandleCommonErrors(w http.ResponseWriter, r *http.Request, err error) {
 	}
 }
 
+func HandleCommonWriteErrors(w http.ResponseWriter, r *http.Request, err error) {
+	switch {
+	case errors.Is(err, ledgercontroller.ErrIdempotencyKeyConflict{}):
+		api.WriteErrorResponse(w, http.StatusConflict, ErrConflict, err)
+	case errors.Is(err, ledgercontroller.ErrInvalidIdempotencyInput{}):
+		api.BadRequest(w, ErrValidation, err)
+	case errors.Is(err, ledgercontroller.ErrNotFound):
+		api.NotFound(w, err)
+	default:
+		HandleCommonErrors(w, r, err)
+	}
+}
+
 func InternalServerError(w http.ResponseWriter, r *http.Request, err error) {
 	otlp.RecordError(r.Context(), err)
 	logging.FromContext(r.Context()).Error(err)
diff --git a/internal/api/v1/controllers_accounts_add_metadata.go b/internal/api/v1/controllers_accounts_add_metadata.go
@@ -2,12 +2,14 @@ package v1
 
 import (
 	"encoding/json"
-	"github.com/formancehq/ledger/internal/controller/ledger"
-	"github.com/formancehq/ledger/pkg/accounts"
 	"net/http"
 	"net/url"
 
+	"github.com/formancehq/ledger/internal/controller/ledger"
+	"github.com/formancehq/ledger/pkg/accounts"
+
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/metadata"
 	"github.com/formancehq/ledger/internal/api/common"
@@ -38,7 +40,7 @@ func addAccountMetadata(w http.ResponseWriter, r *http.Request) {
 		Metadata: m,
 	}))
 	if err != nil {
-		common.HandleCommonErrors(w, r, err)
+		common.HandleCommonWriteErrors(w, r, err)
 		return
 	}
 
diff --git a/internal/api/v1/controllers_accounts_delete_metadata.go b/internal/api/v1/controllers_accounts_delete_metadata.go
@@ -1,10 +1,11 @@
 package v1
 
 import (
-	"github.com/formancehq/ledger/internal/controller/ledger"
 	"net/http"
 	"net/url"
 
+	"github.com/formancehq/ledger/internal/controller/ledger"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/ledger/internal/api/common"
 	"github.com/go-chi/chi/v5"
@@ -25,7 +26,7 @@ func deleteAccountMetadata(w http.ResponseWriter, r *http.Request) {
 				Key:     chi.URLParam(r, "key"),
 			}),
 		); err != nil {
-		common.HandleCommonErrors(w, r, err)
+		common.HandleCommonWriteErrors(w, r, err)
 		return
 	}
 
diff --git a/internal/api/v1/controllers_transactions_add_metadata.go b/internal/api/v1/controllers_transactions_add_metadata.go
@@ -8,6 +8,7 @@ import (
 	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/metadata"
 	"github.com/formancehq/ledger/internal/api/common"
@@ -37,7 +38,7 @@ func addTransactionMetadata(w http.ResponseWriter, r *http.Request) {
 		case errors.Is(err, ledgercontroller.ErrNotFound):
 			api.NotFound(w, err)
 		default:
-			common.HandleCommonErrors(w, r, err)
+			common.HandleCommonWriteErrors(w, r, err)
 		}
 		return
 	}
diff --git a/internal/api/v1/controllers_transactions_create.go b/internal/api/v1/controllers_transactions_create.go
@@ -9,6 +9,7 @@ import (
 	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/metadata"
 	"github.com/formancehq/go-libs/v3/time"
@@ -100,7 +101,7 @@ func createTransaction(w http.ResponseWriter, r *http.Request) {
 			case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}):
 				api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err)
 			default:
-				common.HandleCommonErrors(w, r, err)
+				common.HandleCommonWriteErrors(w, r, err)
 			}
 			return
 		}
@@ -135,7 +136,7 @@ func createTransaction(w http.ResponseWriter, r *http.Request) {
 		case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}):
 			api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err)
 		default:
-			common.HandleCommonErrors(w, r, err)
+			common.HandleCommonWriteErrors(w, r, err)
 		}
 		return
 	}
diff --git a/internal/api/v1/controllers_transactions_delete_metadata.go b/internal/api/v1/controllers_transactions_delete_metadata.go
@@ -7,6 +7,7 @@ import (
 	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/ledger/internal/api/common"
 	"github.com/go-chi/chi/v5"
@@ -27,12 +28,7 @@ func deleteTransactionMetadata(w http.ResponseWriter, r *http.Request) {
 		TransactionID: transactionID,
 		Key:           metadataKey,
 	})); err != nil {
-		switch {
-		case errors.Is(err, ledgercontroller.ErrNotFound):
-			api.NotFound(w, err)
-		default:
-			common.HandleCommonErrors(w, r, err)
-		}
+		common.HandleCommonWriteErrors(w, r, err)
 		return
 	}
 
diff --git a/internal/api/v2/controllers_accounts_add_metadata.go b/internal/api/v2/controllers_accounts_add_metadata.go
@@ -2,11 +2,13 @@ package v2
 
 import (
 	"encoding/json"
-	"github.com/formancehq/ledger/internal/controller/ledger"
 	"net/http"
 	"net/url"
 
+	"github.com/formancehq/ledger/internal/controller/ledger"
+
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/metadata"
 	"github.com/formancehq/ledger/internal/api/common"
@@ -33,7 +35,7 @@ func addAccountMetadata(w http.ResponseWriter, r *http.Request) {
 		Metadata: m,
 	}))
 	if err != nil {
-		common.HandleCommonErrors(w, r, err)
+		common.HandleCommonWriteErrors(w, r, err)
 		return
 	}
 
diff --git a/internal/api/v2/controllers_accounts_delete_metadata.go b/internal/api/v2/controllers_accounts_delete_metadata.go
@@ -1,10 +1,11 @@
 package v2
 
 import (
-	"github.com/formancehq/ledger/internal/controller/ledger"
 	"net/http"
 	"net/url"
 
+	"github.com/formancehq/ledger/internal/controller/ledger"
+
 	"github.com/go-chi/chi/v5"
 
 	"github.com/formancehq/go-libs/v3/api"
@@ -26,7 +27,7 @@ func deleteAccountMetadata(w http.ResponseWriter, r *http.Request) {
 				Key:     chi.URLParam(r, "key"),
 			}),
 		); err != nil {
-		common.HandleCommonErrors(w, r, err)
+		common.HandleCommonWriteErrors(w, r, err)
 		return
 	}
 
diff --git a/internal/api/v2/controllers_ledgers_delete_metadata.go b/internal/api/v2/controllers_ledgers_delete_metadata.go
@@ -1,9 +1,10 @@
 package v2
 
 import (
-	"github.com/formancehq/ledger/internal/api/common"
 	"net/http"
 
+	"github.com/formancehq/ledger/internal/api/common"
+
 	"github.com/formancehq/ledger/internal/controller/system"
 
 	"github.com/formancehq/go-libs/v3/api"
@@ -13,7 +14,7 @@ import (
 func deleteLedgerMetadata(b system.Controller) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		if err := b.DeleteLedgerMetadata(r.Context(), chi.URLParam(r, "ledger"), chi.URLParam(r, "key")); err != nil {
-			common.HandleCommonErrors(w, r, err)
+			common.HandleCommonWriteErrors(w, r, err)
 			return
 		}
 
diff --git a/internal/api/v2/controllers_ledgers_update_metadata.go b/internal/api/v2/controllers_ledgers_update_metadata.go
@@ -2,10 +2,12 @@ package v2
 
 import (
 	"encoding/json"
-	"github.com/formancehq/ledger/internal/api/common"
 	"net/http"
 
+	"github.com/formancehq/ledger/internal/api/common"
+
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/metadata"
 	systemcontroller "github.com/formancehq/ledger/internal/controller/system"
@@ -22,7 +24,7 @@ func updateLedgerMetadata(systemController systemcontroller.Controller) http.Han
 		}
 
 		if err := systemController.UpdateLedgerMetadata(r.Context(), chi.URLParam(r, "ledger"), m); err != nil {
-			common.HandleCommonErrors(w, r, err)
+			common.HandleCommonWriteErrors(w, r, err)
 			return
 		}
 
diff --git a/internal/api/v2/controllers_transactions_add_metadata.go b/internal/api/v2/controllers_transactions_add_metadata.go
@@ -8,6 +8,7 @@ import (
 	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/go-libs/v3/metadata"
 	"github.com/formancehq/ledger/internal/api/common"
@@ -37,7 +38,7 @@ func addTransactionMetadata(w http.ResponseWriter, r *http.Request) {
 		case errors.Is(err, ledgercontroller.ErrNotFound):
 			api.NotFound(w, err)
 		default:
-			common.HandleCommonErrors(w, r, err)
+			common.HandleCommonWriteErrors(w, r, err)
 		}
 		return
 	}
diff --git a/internal/api/v2/controllers_transactions_create.go b/internal/api/v2/controllers_transactions_create.go
@@ -2,9 +2,10 @@ package v2
 
 import (
 	"encoding/json"
-	"github.com/formancehq/ledger/internal/api/bulking"
 	"net/http"
 
+	"github.com/formancehq/ledger/internal/api/bulking"
+
 	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 
 	"errors"
@@ -51,14 +52,12 @@ func createTransaction(w http.ResponseWriter, r *http.Request) {
 			api.BadRequest(w, common.ErrNoPostings, err)
 		case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}):
 			api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err)
-		case errors.Is(err, ledgercontroller.ErrInvalidIdempotencyInput{}):
-			api.BadRequest(w, common.ErrValidation, err)
 		case errors.Is(err, ledgercontroller.ErrParsing{}):
 			api.BadRequest(w, common.ErrInterpreterParse, err)
 		case errors.Is(err, ledgercontroller.ErrRuntime{}):
 			api.BadRequest(w, common.ErrInterpreterRuntime, err)
 		default:
-			common.HandleCommonErrors(w, r, err)
+			common.HandleCommonWriteErrors(w, r, err)
 		}
 		return
 	}
diff --git a/internal/api/v2/controllers_transactions_delete_metadata.go b/internal/api/v2/controllers_transactions_delete_metadata.go
@@ -8,7 +8,6 @@ import (
 
 	"github.com/go-chi/chi/v5"
 
-	"errors"
 	"github.com/formancehq/ledger/internal/api/common"
 
 	"github.com/formancehq/go-libs/v3/api"
@@ -29,12 +28,7 @@ func deleteTransactionMetadata(w http.ResponseWriter, r *http.Request) {
 		TransactionID: txID,
 		Key:           metadataKey,
 	})); err != nil {
-		switch {
-		case errors.Is(err, ledgercontroller.ErrNotFound):
-			api.NotFound(w, err)
-		default:
-			common.HandleCommonErrors(w, r, err)
-		}
+		common.HandleCommonWriteErrors(w, r, err)
 		return
 	}
 
diff --git a/internal/api/v2/controllers_transactions_revert.go b/internal/api/v2/controllers_transactions_revert.go
@@ -7,6 +7,7 @@ import (
 	ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"
 
 	"errors"
+
 	"github.com/formancehq/go-libs/v3/api"
 	"github.com/formancehq/ledger/internal/api/common"
 	"github.com/go-chi/chi/v5"
diff --git a/openapi.yaml b/openapi.yaml
@@ -1864,6 +1864,11 @@ paths:
           schema:
             type: string
             example: foo
+        - name: Idempotency-Key
+          in: header
+          description: Use an idempotency key
+          schema:
+            type: string
       responses:
         2XX:
           description: Key deleted
@@ -2241,6 +2246,11 @@ paths:
           schema:
             type: string
             example: foo
+        - name: Idempotency-Key
+          in: header
+          description: Use an idempotency key
+          schema:
+            type: string
       responses:
         2XX:
           description: Key deleted
diff --git a/openapi/v2.yaml b/openapi/v2.yaml
@@ -604,6 +604,11 @@ paths:
           schema:
             type: string
             example: foo
+        - name: Idempotency-Key
+          in: header
+          description: Use an idempotency key
+          schema:
+            type: string
       responses:
         2XX:
           description: Key deleted
@@ -983,6 +988,11 @@ paths:
           schema:
             type: string
             example: foo
+        - name: Idempotency-Key
+          in: header
+          description: Use an idempotency key
+          schema:
+            type: string
       responses:
         2XX:
           description: Key deleted
diff --git a/pkg/client/.speakeasy/gen.lock b/pkg/client/.speakeasy/gen.lock
diff --git a/pkg/client/docs/models/operations/v2deleteaccountmetadatarequest.md b/pkg/client/docs/models/operations/v2deleteaccountmetadatarequest.md
diff --git a/pkg/client/docs/models/operations/v2deletetransactionmetadatarequest.md b/pkg/client/docs/models/operations/v2deletetransactionmetadatarequest.md
diff --git a/pkg/client/models/operations/v2deleteaccountmetadata.go b/pkg/client/models/operations/v2deleteaccountmetadata.go
diff --git a/pkg/client/models/operations/v2deletetransactionmetadata.go b/pkg/client/models/operations/v2deletetransactionmetadata.go
diff --git a/pkg/client/v2.go b/pkg/client/v2.go
diff --git a/test/e2e/api_accounts_metadata_test.go b/test/e2e/api_accounts_metadata_test.go
diff --git a/test/e2e/api_idempotency_test.go b/test/e2e/api_idempotency_test.go
diff --git a/test/e2e/api_transactions_metadata_test.go b/test/e2e/api_transactions_metadata_test.go

Original file line number	Diff line number	Diff line change
`@@ -9,6 +9,7 @@ import (`
`9`	`9`	`ledgercontroller "github.com/formancehq/ledger/internal/controller/ledger"`
`10`	`10`
`11`	`11`	`"errors"`
	`12`	`+`
`12`	`13`	`"github.com/formancehq/go-libs/v3/api"`
`13`	`14`	`"github.com/formancehq/go-libs/v3/metadata"`
`14`	`15`	`"github.com/formancehq/go-libs/v3/time"`
`@@ -100,7 +101,7 @@ func createTransaction(w http.ResponseWriter, r *http.Request) {`
`100`	`101`	`case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}):`
`101`	`102`	`api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err)`
`102`	`103`	`default:`
`103`		`- common.HandleCommonErrors(w, r, err)`
	`104`	`+ common.HandleCommonWriteErrors(w, r, err)`
`104`	`105`	`}`
`105`	`106`	`return`
`106`	`107`	`}`
`@@ -135,7 +136,7 @@ func createTransaction(w http.ResponseWriter, r *http.Request) {`
`135`	`136`	`case errors.Is(err, ledgercontroller.ErrTransactionReferenceConflict{}):`
`136`	`137`	`api.WriteErrorResponse(w, http.StatusConflict, common.ErrConflict, err)`
`137`	`138`	`default:`
`138`		`- common.HandleCommonErrors(w, r, err)`
	`139`	`+ common.HandleCommonWriteErrors(w, r, err)`
`139`	`140`	`}`
`140`	`141`	`return`
`141`	`142`	`}`