Merge pull request #524 from velzevur/PT-153629617_HTTP_ext_api_retur…

…n_codes Revisit HTTP API endpoint error codes [Finishes # 153629617]
aeternity · Dec 21, 2017 · 9e67a1a · 9e67a1a
2 parents 631e765 + d7043c8
commit 9e67a1a
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 19 deletions.
diff --git a/apps/aehttp/priv/swagger.json b/apps/aehttp/priv/swagger.json
@@ -41,7 +41,13 @@
               "$ref" : "#/definitions/Ping"
             }
           },
-          "404" : {
+          "403" : {
+            "description" : "Not allowed (node is blocked)",
+            "schema" : {
+              "$ref" : "#/definitions/Error"
+            }
+          },
+          "409" : {
             "description" : "Different genesis blocks",
             "schema" : {
               "$ref" : "#/definitions/Error"
@@ -152,7 +158,7 @@
           "200" : {
             "description" : "successful operation"
           },
-          "404" : {
+          "400" : {
             "description" : "Block or header validation error",
             "schema" : {
               "$ref" : "#/definitions/Error"

diff --git a/apps/aehttp/src/aehttp_dispatch_ext.erl b/apps/aehttp/src/aehttp_dispatch_ext.erl
@@ -90,7 +90,7 @@ handle_request('PostBlock', Req, _Context) ->
         ok -> {200, [], #{}};
         {error, Reason} ->
             lager:error("Post block failed: ~p", [Reason]),
-            {200, [], #{}}
+            {400, [], #{reason => <<"Block rejected">>}}
     end;
 
 handle_request('PostTx', #{'Tx' := Tx} = Req, _Context) ->
@@ -211,19 +211,16 @@ handle_ping(#{<<"source">> := Src} = PingObj) ->
     case IsBlocked of
         false -> handle_ping_(PingObj);
         true  ->
-            abort_sync(Src, <<"Not allowed">>)
-    end;
-handle_ping(_) ->
-    Reason = <<"Missing source attribute">>,
-    abort_sync(undefined, Reason).
+            abort_sync(Src, 403, <<"Not allowed">>)
+    end.
 
 handle_ping_(PingObj) ->
     LocalPingObj = aec_sync:local_ping_object(),
     case aec_sync:compare_ping_objects(LocalPingObj, PingObj) of
         {error, different_genesis_blocks} ->
             Source = maps:get(<<"source">>, PingObj),
             aec_peers:block_peer(Source),
-            abort_sync(Source, <<"Different genesis blocks">>);
+            abort_sync(Source, 409, <<"Different genesis blocks">>);
         ok ->
             Source = maps:get(<<"source">>, PingObj),
             aec_peers:update_last_seen(Source),
@@ -244,9 +241,9 @@ handle_ping_(PingObj) ->
             {200, [], Res}
     end.
 
-abort_sync(Uri, Reason) ->
+abort_sync(Uri, Code, Reason) ->
     aec_events:publish(
       chain_sync,
       {sync_aborted, #{uri => Uri,
                        reason => Reason}}),
-      {404, [], #{reason => Reason}}.
+      {Code, [], #{reason => Reason}}.
diff --git a/apps/aehttp/src/swagger/swagger_api.erl b/apps/aehttp/src/swagger/swagger_api.erl
@@ -331,12 +331,14 @@ validate_response('GetTxs', 200, Body, ValidatorState) ->
 
 validate_response('Ping', 200, Body, ValidatorState) ->
     validate_response_body('Ping', 'Ping', Body, ValidatorState);
-validate_response('Ping', 404, Body, ValidatorState) ->
+validate_response('Ping', 403, Body, ValidatorState) ->
+    validate_response_body('Error', 'Error', Body, ValidatorState);
+validate_response('Ping', 409, Body, ValidatorState) ->
     validate_response_body('Error', 'Error', Body, ValidatorState);
 
 validate_response('PostBlock', 200, Body, ValidatorState) ->
     validate_response_body('', '', Body, ValidatorState);
-validate_response('PostBlock', 404, Body, ValidatorState) ->
+validate_response('PostBlock', 400, Body, ValidatorState) ->
     validate_response_body('Error', 'Error', Body, ValidatorState);
 
 

diff --git a/apps/aehttp/test/aehttp_integration_SUITE.erl b/apps/aehttp/test/aehttp_integration_SUITE.erl
@@ -158,7 +158,7 @@ broken_pings(_Config) ->
     % wrong genesis hash 
     WrongGenHashPing = maps:merge(PingObj, #{<<"source">> => unique_peer(),
                                              <<"genesis_hash">> => <<"foo">>}),
-    {ok, 404, #{<<"reason">> := <<"Different genesis blocks">>}} =
+    {ok, 409, #{<<"reason">> := <<"Different genesis blocks">>}} =
         post_ping(WrongGenHashPing),
     ok.
 
@@ -167,9 +167,9 @@ blocked_ping(_Config) ->
     PingObj = rpc(aec_sync, local_ping_object, []),
     WrongGenHashPing = maps:merge(PingObj, #{<<"source">> => Peer,
                                              <<"genesis_hash">> => <<"foo">>}),
-    {ok, 404, _} = post_ping(WrongGenHashPing),
+    {ok, 409, _} = post_ping(WrongGenHashPing),
     % node is blocked now 
-    {ok, 404, #{<<"reason">> := <<"Not allowed">>}} =
+    {ok, 403, #{<<"reason">> := <<"Not allowed">>}} =
         post_ping(maps:put(<<"source">>, Peer, PingObj)),
     ok.
 
@@ -388,7 +388,7 @@ post_broken_blocks(Config) ->
         fun({BrokenField, BlockMap}) ->
             ct:log("Testing with a broken ~p", [BrokenField]),
             {ok, Block} = aec_blocks:deserialize_from_map(BlockMap),
-            {ok, 200, _} = post_block(Block),
+            {ok, 400, #{<<"reason">> := <<"Block rejected">>}} = post_block(Block),
             H = rpc(aec_conductor, top_header, []), 
             0 = aec_headers:height(H) %chain is still empty
         end,

diff --git a/config/swagger.yaml b/config/swagger.yaml
@@ -41,7 +41,11 @@ paths:
           description: Sample response
           schema:
             $ref: '#/definitions/Ping'
-        '404':
+        '403':
+          description: Not allowed (node is blocked) 
+          schema:
+            $ref: '#/definitions/Error'
+        '409':
           description: Different genesis blocks
           schema:
             $ref: '#/definitions/Error'
@@ -135,7 +139,7 @@ paths:
       responses:
         '200':
           description: successful operation
-        '404':
+        '400':
           description: Block or header validation error
           schema:
             $ref: '#/definitions/Error'