From 15594b4db64e44d1473fd0b62f53827087644d4e Mon Sep 17 00:00:00 2001
From: Kjell Winblad <kjellwinblad@gmail.com>
Date: Thu, 25 Apr 2024 16:54:09 +0200
Subject: [PATCH 1/2] fix(HTTP connector): retry on 503 Service Unavailable
 response

Previously, if an HTTP request received a 503 (Service Unavailable)
status, it was marked as a failure without retrying. This has now been
fixed so that the request is retried a configurable number of times.

Fixes:
https://emqx.atlassian.net/browse/EMQX-12217
https://github.com/emqx/emqx/issues/12869 (partly)
---
 .../src/emqx_bridge_http_connector.erl        |  7 +++++
 .../test/emqx_bridge_http_SUITE.erl           | 31 +++++++++++++++++--
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/apps/emqx_bridge_http/src/emqx_bridge_http_connector.erl b/apps/emqx_bridge_http/src/emqx_bridge_http_connector.erl
index 88e4d9c36..ef9c6c70d 100644
--- a/apps/emqx_bridge_http/src/emqx_bridge_http_connector.erl
+++ b/apps/emqx_bridge_http/src/emqx_bridge_http_connector.erl
@@ -836,6 +836,8 @@ transform_result(Result) ->
             Result;
         {ok, _TooManyRequests = StatusCode = 429, Headers} ->
             {error, {recoverable_error, #{status_code => StatusCode, headers => Headers}}};
+        {ok, _ServiceUnavailable = StatusCode = 503, Headers} ->
+            {error, {recoverable_error, #{status_code => StatusCode, headers => Headers}}};
         {ok, StatusCode, Headers} ->
             {error, {unrecoverable_error, #{status_code => StatusCode, headers => Headers}}};
         {ok, _TooManyRequests = StatusCode = 429, Headers, Body} ->
@@ -843,6 +845,11 @@ transform_result(Result) ->
                 {recoverable_error, #{
                     status_code => StatusCode, headers => Headers, body => Body
                 }}};
+        {ok, _ServiceUnavailable = StatusCode = 503, Headers, Body} ->
+            {error,
+                {recoverable_error, #{
+                    status_code => StatusCode, headers => Headers, body => Body
+                }}};
         {ok, StatusCode, Headers, Body} ->
             {error,
                 {unrecoverable_error, #{
diff --git a/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl b/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl
index 9d215d815..7f9418bd3 100644
--- a/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl
+++ b/apps/emqx_bridge_http/test/emqx_bridge_http_SUITE.erl
@@ -93,6 +93,14 @@ init_per_testcase(t_too_many_requests, Config) ->
     ),
     ok = emqx_bridge_http_connector_test_server:set_handler(too_many_requests_http_handler()),
     [{http_server, #{port => HTTPPort, path => HTTPPath}} | Config];
+init_per_testcase(t_service_unavailable, Config) ->
+    HTTPPath = <<"/path">>,
+    ServerSSLOpts = false,
+    {ok, {HTTPPort, _Pid}} = emqx_bridge_http_connector_test_server:start_link(
+        _Port = random, HTTPPath, ServerSSLOpts
+    ),
+    ok = emqx_bridge_http_connector_test_server:set_handler(service_unavailable_http_handler()),
+    [{http_server, #{port => HTTPPort, path => HTTPPath}} | Config];
 init_per_testcase(t_rule_action_expired, Config) ->
     [
         {bridge_name, ?BRIDGE_NAME}
@@ -115,6 +123,7 @@ init_per_testcase(_TestCase, Config) ->
 end_per_testcase(TestCase, _Config) when
     TestCase =:= t_path_not_found;
     TestCase =:= t_too_many_requests;
+    TestCase =:= t_service_unavailable;
     TestCase =:= t_rule_action_expired;
     TestCase =:= t_bridge_probes_header_atoms;
     TestCase =:= t_send_async_connection_timeout;
@@ -260,6 +269,12 @@ not_found_http_handler() ->
     end.
 
 too_many_requests_http_handler() ->
+    fail_then_success_http_handler(429).
+
+service_unavailable_http_handler() ->
+    fail_then_success_http_handler(503).
+
+fail_then_success_http_handler(FailStatusCode) ->
     GetAndBump =
         fun() ->
             NCalled = persistent_term:get({?MODULE, times_called}, 0),
@@ -272,7 +287,7 @@ too_many_requests_http_handler() ->
         {ok, Body, Req} = cowboy_req:read_body(Req0),
         TestPid ! {http, cowboy_req:headers(Req), Body},
         Rep =
-            case N >= 2 of
+            case N >= 3 of
                 true ->
                     cowboy_req:reply(
                         200,
@@ -282,9 +297,13 @@ too_many_requests_http_handler() ->
                     );
                 false ->
                     cowboy_req:reply(
-                        429,
+                        FailStatusCode,
                         #{<<"content-type">> => <<"text/plain">>},
-                        <<"slow down, buddy">>,
+                        %% Body and no body to trigger different code paths
+                        case N of
+                            1 -> <<"slow down, buddy">>;
+                            _ -> <<>>
+                        end,
                         Req
                     )
             end,
@@ -570,6 +589,12 @@ t_path_not_found(Config) ->
     ok.
 
 t_too_many_requests(Config) ->
+    check_send_is_retried(Config).
+
+t_service_unavailable(Config) ->
+    check_send_is_retried(Config).
+
+check_send_is_retried(Config) ->
     ?check_trace(
         begin
             #{port := Port, path := Path} = ?config(http_server, Config),

From 6ab9460c1657b9b51c1e1785a0b5e8dabad66f5b Mon Sep 17 00:00:00 2001
From: Kjell Winblad <kjellwinblad@gmail.com>
Date: Thu, 25 Apr 2024 17:10:30 +0200
Subject: [PATCH 2/2] docs: add change log entry

---
 changes/ce/fix-12932.en.md | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 changes/ce/fix-12932.en.md

diff --git a/changes/ce/fix-12932.en.md b/changes/ce/fix-12932.en.md
new file mode 100644
index 000000000..fee7078a7
--- /dev/null
+++ b/changes/ce/fix-12932.en.md
@@ -0,0 +1 @@
+Previously, if a HTTP action request received a 503 (Service Unavailable) status, it was marked as a failure and the request was not retried. This has now been fixed so that the request is retried a configurable number of times.