errors: add a catch-all handler (#12)

The main goal of this PR is in the first commit: ## errors: add a catch-all handler to give feedback to the proxy user This generic error handler helps to give context in case of target server errors. Indeed without this commit, the user of the proxy would see a `500 Internal server error`. Now, with the current change the real error is catched and a 502 HTTP error is responded to the user, with some details in a json object `{error: error.message}`. E.g. when targeting a server which has a bad SSL certificate, the user will now receive this response: ``` HTTP/1.1 502 Bad Gateway access-control-allow-origin: * access-control-allow-methods: OPTIONS access-control-allow-methods: GET access-control-allow-methods: POST access-control-allow-methods: PUT access-control-allow-methods: PATCH access-control-allow-methods: DELETE access-control-allow-headers: Content-Type, Authorization, x-bump-proxy-token, x-requested-with content-type: application/json x-content-type-options: nosniff Content-Length: 127 {"error":"SSL_connect returned=1 errno=0 peeraddr=212.95.74.75:443 state=error: certificate verify failed (hostname mismatch)"} ``` ## Other changes - Make sure to return correct content-type when JSON errors are returned - Refactoring of the test file to clarify the context / stubs
bump-sh · Nov 26, 2024 · de04a4a · de04a4a
1 parent 1046d84
commit de04a4a
Show file tree

Hide file tree

Showing 2 changed files with 101 additions and 72 deletions.
diff --git a/proxy_server.rb b/proxy_server.rb
@@ -20,17 +20,25 @@ class ProxyServer < Sinatra::Base
   ).freeze
 
   error JWT::ExpiredSignature do
+    headers "Content-Type" => "application/json"
     halt 401, {error: "Token has expired"}.to_json
   end
 
   error JWT::DecodeError do
+    headers "Content-Type" => "application/json"
     halt 401, {error: "Invalid token"}.to_json
   end
 
   error JWT::MissingRequiredClaim do |error|
+    headers "Content-Type" => "application/json"
     halt 401, {error: "Token has #{error.to_s.downcase}"}.to_json
   end
 
+  error do |error|
+    headers "Content-Type" => "application/json"
+    halt 502, {error: error.message}.to_json
+  end
+
   # Handle CORS headers
   before do
     headers "Access-Control-Allow-Origin" => "*",

diff --git a/spec/proxy_server_spec.rb b/spec/proxy_server_spec.rb
@@ -13,7 +13,7 @@
   include Rack::Test::Methods
 
   def app
-    ProxyServer
+    @app ||= ProxyServer
   end
 
   def expect_header(k, v)
@@ -52,31 +52,10 @@ def expect_json_body(k, v)
 
   let(:target_url) { "https://jsonplaceholder.typicode.com/posts" }
 
-  # Mock external requests with WebMock or a similar tool (if desired)
-
   before(:each) do
     stub_request(:get, "https://jsonplaceholder.typicode.com/posts")
       .with(headers: {"x-foo": "bar"})
       .to_return(status: 200, body: "", headers: {})
-    stub_request(:put, "https://jsonplaceholder.typicode.com/posts/1")
-      .to_return(status: 200, body: {title: "updated title"}.to_json, headers: {})
-    stub_request(:post, "https://jsonplaceholder.typicode.com/posts")
-      .to_return(status: 201, body: {title: "foo", body: "bar", userId: 1}.to_json, headers: {})
-    ["https://staging.bump.sh/api/v1/ping", "https://bump.sh/api/Custom+Api/v1/ping"].each do |server|
-      stub_request(:get, server)
-        .with(
-          headers: {
-            'Accept' => '*/*',
-            'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
-            'Cookie' => '',
-            'Host' => URI.parse(server).host,
-            'User-Agent' => 'Ruby',
-            'X-Foo' => 'bar'
-          },
-          query: hash_including # Allow any query parameters
-        )
-        .to_return(status: 200, body: "", headers: {})
-    end
   end
 
   context "preflight request" do
@@ -93,6 +72,21 @@ def expect_json_body(k, v)
     context "and is valid" do
       context "when no path params" do
         before(:each) do
+          ["https://staging.bump.sh/api/v1/ping", "https://bump.sh/api/Custom+Api/v1/ping"].each do |server|
+            stub_request(:get, server)
+              .with(
+                headers: {
+                  'Accept' => '*/*',
+                  'Accept-Encoding' => 'gzip;q=1.0,deflate;q=0.6,identity;q=0.3',
+                  'Cookie' => '',
+                  'Host' => URI.parse(server).host,
+                  'User-Agent' => 'Ruby',
+                  'X-Foo' => 'bar'
+                },
+                query: hash_including # Allow any query parameters
+              )
+              .to_return(status: 200, body: "", headers: {})
+          end
           header "x-bump-proxy-token", proxy_token
           header "x-foo", "bar"
           get "/#{target_url}"
@@ -187,6 +181,83 @@ def expect_json_body(k, v)
           expect_header("access-control-allow-origin", "*")
         end
       end
+
+      context "when POST requests" do
+        let(:verb) { "POST" }
+        let(:request_body) { {title: "foo", body: "bar", userId: 1} }
+
+        before(:each) do
+          stub_request(:post, "https://jsonplaceholder.typicode.com/posts")
+            .to_return(status: 201, body: {title: "foo", body: "bar", userId: 1}.to_json, headers: {})
+          header "x-bump-proxy-token", proxy_token
+          header "Content-Type", "application/json"
+          post "/#{target_url}", request_body.to_json
+        end
+
+        it "returns a 201 Created status" do
+          expect(last_response.status).to eq(201)
+        end
+
+        it "includes CORS headers in the response" do
+          expect_header("access-control-allow-origin", "*")
+        end
+
+        it "returns the correct title in the response body" do
+          response_body = JSON.parse(last_response.body)
+          expect(response_body["title"]).to eq("foo")
+        end
+
+        it "returns the correct body in the response body" do
+          response_body = JSON.parse(last_response.body)
+          expect(response_body["body"]).to eq("bar")
+        end
+
+        it "returns the correct userId in the response body" do
+          response_body = JSON.parse(last_response.body)
+          expect(response_body["userId"]).to eq(1)
+        end
+      end
+
+      context "when PUT requests" do
+        let(:verb) { "PUT" }
+        let(:path) { "/posts/{id}" }
+
+        before(:each) do
+          stub_request(:put, "https://jsonplaceholder.typicode.com/posts/1")
+            .to_return(status: 200, body: {title: "updated title"}.to_json, headers: {})
+          header "x-bump-proxy-token", proxy_token
+          header "Content-Type", "application/json"
+          put "/#{target_url}/1", {id: 1, title: "updated title"}.to_json
+        end
+
+        it "forwards headers and body for PUT requests" do
+          expect(last_response.status).to eq(200)  # Expect OK status if target server responds as expected
+          response_body = JSON.parse(last_response.body)
+          expect(response_body["title"]).to eq("updated title")
+        end
+      end
+
+      context "when target request returns an error" do
+        before(:each) do
+          # This sinatra config setting simulates the production
+          # behavior (because in dev/test the generic error handler is
+          # not called, instead errors are raised for real)
+          @app = Sinatra.new(ProxyServer) do
+            set :raise_errors, false
+          end
+          stub_request(:get, "https://jsonplaceholder.typicode.com/posts")
+            .to_raise(OpenSSL::SSL::SSLError)
+          header "x-bump-proxy-token", proxy_token
+          header "Content-Type", "application/json"
+          get "/#{target_url}"
+        end
+
+        it "returns a 502 and forwards the error message" do
+          expect(last_response.status).to eq(502)
+          response_body = JSON.parse(last_response.body)
+          expect(response_body["error"]).to eq("Exception from WebMock")
+        end
+      end
     end
 
     context "but is invalid" do
@@ -342,56 +413,6 @@ def expect_json_body(k, v)
   end
 
   context "request forwarding" do
-    context "when POST requests" do
-      let(:verb) { "POST" }
-      let(:request_body) { {title: "foo", body: "bar", userId: 1} }
-
-      before(:each) do
-        header "x-bump-proxy-token", proxy_token
-        header "Content-Type", "application/json"
-        post "/#{target_url}", request_body.to_json
-      end
-
-      it "returns a 201 Created status" do
-        expect(last_response.status).to eq(201)
-      end
-
-      it "includes CORS headers in the response" do
-        expect_header("access-control-allow-origin", "*")
-      end
-
-      it "returns the correct title in the response body" do
-        response_body = JSON.parse(last_response.body)
-        expect(response_body["title"]).to eq("foo")
-      end
-
-      it "returns the correct body in the response body" do
-        response_body = JSON.parse(last_response.body)
-        expect(response_body["body"]).to eq("bar")
-      end
-
-      it "returns the correct userId in the response body" do
-        response_body = JSON.parse(last_response.body)
-        expect(response_body["userId"]).to eq(1)
-      end
-    end
-
-    context "when PUT requests" do
-      let(:verb) { "PUT" }
-      let(:path) { "/posts/{id}" }
-
-      before(:each) do
-        header "x-bump-proxy-token", proxy_token
-        header "Content-Type", "application/json"
-        put "/#{target_url}/1", {id: 1, title: "updated title"}.to_json
-      end
-
-      it "forwards headers and body for PUT requests" do
-        expect(last_response.status).to eq(200)  # Expect OK status if target server responds as expected
-        response_body = JSON.parse(last_response.body)
-        expect(response_body["title"]).to eq("updated title")
-      end
-    end
   end
 
   context "startup of ProxyServer" do