From aa080b4130226b864adcfecd8fb02a4f79ff58a9 Mon Sep 17 00:00:00 2001
From: "Paul B." <paulr@bump.sh>
Date: Mon, 25 Nov 2024 16:48:47 +0100
Subject: [PATCH] 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)"}
```
---
 proxy_server.rb           |  4 ++++
 spec/proxy_server_spec.rb | 24 +++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/proxy_server.rb b/proxy_server.rb
index 4df415b..1ca8b20 100644
--- a/proxy_server.rb
+++ b/proxy_server.rb
@@ -31,6 +31,10 @@ class ProxyServer < Sinatra::Base
     halt 401, {error: "Token has #{error.to_s.downcase}"}.to_json
   end
 
+  error do |error|
+    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
index 23a3cd4..efeb289 100644
--- 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)
@@ -187,6 +187,28 @@ def expect_json_body(k, v)
           expect_header("access-control-allow-origin", "*")
         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