From 6b265baf1b233846ce13912d200ab0734f0adea4 Mon Sep 17 00:00:00 2001 From: Antoine Gelloz Date: Wed, 9 Nov 2022 16:54:50 +0100 Subject: [PATCH] fix: Script controller conflict on reference & storage conflict error code 409 instead of 503 (#360) * fix: script controller conflict on reference * fix: storage conflict error code 409 instead of 503 * fix: update swagger --- pkg/api/controllers/base_controller.go | 3 +- pkg/api/controllers/script_controller.go | 18 +++-- pkg/api/controllers/script_controller_test.go | 42 +++++++++++ pkg/api/controllers/swagger.yaml | 73 ++++++++++++++++++- 4 files changed, 128 insertions(+), 8 deletions(-) diff --git a/pkg/api/controllers/base_controller.go b/pkg/api/controllers/base_controller.go index 5233ae335..94618dcef 100644 --- a/pkg/api/controllers/base_controller.go +++ b/pkg/api/controllers/base_controller.go @@ -41,7 +41,8 @@ const ( func coreErrorToErrorCode(err error) (int, string) { switch { - case ledger.IsConflictError(err): + case ledger.IsConflictError(err) || + storage.IsErrorCode(err, storage.ConstraintFailed): return http.StatusConflict, ErrConflict case ledger.IsInsufficientFundError(err): return http.StatusBadRequest, ErrInsufficientFund diff --git a/pkg/api/controllers/script_controller.go b/pkg/api/controllers/script_controller.go index 411566c87..9f3a5ff4a 100644 --- a/pkg/api/controllers/script_controller.go +++ b/pkg/api/controllers/script_controller.go @@ -43,7 +43,9 @@ func (ctl *ScriptController) PostScript(c *gin.Context) { var script core.Script if err := c.ShouldBindJSON(&script); err != nil { - panic(err) + ResponseError(c, ledger.NewValidationError( + "invalid payload")) + return } value, ok := c.GetQuery("preview") @@ -61,13 +63,17 @@ func (ctl *ScriptController) PostScript(c *gin.Context) { code = ErrInternal message string ) - scriptError, ok := err.(*ledger.ScriptError) - if ok { - code = scriptError.Code - message = scriptError.Message - } else { + switch e := err.(type) { + case *ledger.ScriptError: + code = e.Code + message = e.Message + case *ledger.ConflictError: + code = ErrConflict + message = e.Error() + default: sharedlogging.GetLogger(c.Request.Context()).Errorf("internal errors executing script: %s", err) } + res.ErrorResponse = sharedapi.ErrorResponse{ ErrorCode: code, ErrorMessage: message, diff --git a/pkg/api/controllers/script_controller_test.go b/pkg/api/controllers/script_controller_test.go index 380943fab..a34decfba 100644 --- a/pkg/api/controllers/script_controller_test.go +++ b/pkg/api/controllers/script_controller_test.go @@ -183,3 +183,45 @@ func TestPostScriptWithReference(t *testing.T) { }) })) } + +func TestPostScriptConflict(t *testing.T) { + script := ` + send [COIN 100] ( + source = @world + destination = @centralbank + )` + + internal.RunTest(t, fx.Invoke(func(lc fx.Lifecycle, api *api.API, driver storage.Driver) { + lc.Append(fx.Hook{ + OnStart: func(ctx context.Context) error { + t.Run("first should succeed", func(t *testing.T) { + rsp := internal.PostScript(t, api, core.Script{ + Plain: script, + Reference: "1234", + }, url.Values{}) + + assert.Equal(t, http.StatusOK, rsp.Result().StatusCode) + res := controllers.ScriptResponse{} + internal.Decode(t, rsp.Body, &res) + assert.Equal(t, "", res.ErrorCode) + assert.Equal(t, "", res.ErrorMessage) + assert.NotNil(t, res.Transaction) + }) + + t.Run("second should fail", func(t *testing.T) { + rsp := internal.PostScript(t, api, core.Script{ + Plain: script, + Reference: "1234", + }, url.Values{}) + + assert.Equal(t, http.StatusOK, rsp.Result().StatusCode) + res := controllers.ScriptResponse{} + internal.Decode(t, rsp.Body, &res) + assert.Equal(t, controllers.ErrConflict, res.ErrorCode) + }) + + return nil + }, + }) + })) +} diff --git a/pkg/api/controllers/swagger.yaml b/pkg/api/controllers/swagger.yaml index 49795e985..599a6ee81 100644 --- a/pkg/api/controllers/swagger.yaml +++ b/pkg/api/controllers/swagger.yaml @@ -272,6 +272,20 @@ paths: error_message: type: string example: "invalid account address format" + "409": + description: Conflict + content: + application/json: + schema: + type: object + required: + - error_code + properties: + error_code: + type: string + example: "CONFLICT" + error_message: + type: string /{ledger}/mapping: get: @@ -354,7 +368,36 @@ paths: application/json: schema: $ref: '#/components/schemas/ScriptResult' - + "400": + description: Bad Request + content: + application/json: + schema: + type: object + required: + - error_code + properties: + error_code: + type: string + example: "VALIDATION" + error_message: + type: string + example: "invalid payload" + "409": + description: Conflict + content: + application/json: + schema: + type: object + required: + - error_code + properties: + error_code: + type: string + example: "CONFLICT" + error_message: + type: string + /{ledger}/stats: get: tags: @@ -749,6 +792,20 @@ paths: error_message: type: string example: "transaction not found" + "409": + description: Conflict + content: + application/json: + schema: + type: object + required: + - error_code + properties: + error_code: + type: string + example: "CONFLICT" + error_message: + type: string /{ledger}/transactions/{txid}/revert: post: @@ -808,6 +865,20 @@ paths: error_message: type: string example: "transaction not found" + "409": + description: Conflict + content: + application/json: + schema: + type: object + required: + - error_code + properties: + error_code: + type: string + example: "CONFLICT" + error_message: + type: string /{ledger}/transactions/batch: post: