Merge pull request #58 from fmops/niole.use_repo_helper

Meaningful error on trailing slash
fmops · Nov 18, 2024 · 69a8ac2 · 69a8ac2
2 parents 7b92297 + aa02343
commit 69a8ac2
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 31 deletions.
diff --git a/lib/excision_web/controllers/classifier_controller.ex b/lib/excision_web/controllers/classifier_controller.ex
@@ -186,37 +186,37 @@ defmodule ExcisionWeb.ClassifierController do
         preserve_host_header: false
       )
 
-    resp =
+    conn =
       conn
       # strip path, by default this is appended when proxying
       |> Map.put(:path_info, [])
       |> ReverseProxyPlug.call(opts)
 
     # parse response and record decision
     resp_body =
-      case Plug.Conn.get_resp_header(resp, "content-encoding") do
-        ["gzip"] ->
-          {:ok, :zlib.gunzip(resp.resp_body)}
-
-        ["br"] ->
-          :brotli.decode(resp.resp_body)
-
-        _ ->
-          if resp.status >= 400 do
-            Logger.error(
-              "Got failure response while proxying request for classifier #{classifier.name}: #{resp.status} #{resp.resp_body}"
-            )
-
-            {:error, resp.resp_body}
-          else
-            {:ok, resp.resp_body}
-          end
-      end
+      if conn.status >= 400 do
+        Logger.error(
+          "Got failure response while proxying request for classifier #{classifier.name}: #{conn.status} #{conn.resp_body}"
+        )
+
+        {:error, conn.resp_body}
+      else
+        case Plug.Conn.get_resp_header(conn, "content-encoding") do
+          ["gzip"] ->
+            {:ok, :zlib.gunzip(conn.resp_body)}
 
-    deserialized_body = resp_body |> elem(1) |> Jason.decode!()
+          ["br"] ->
+            :brotli.decode(conn.resp_body)
+
+          _ ->
+            {:ok, conn.resp_body}
+        end
+      end
 
     case resp_body do
       {:ok, _} ->
+        deserialized_body = resp_body |> elem(1) |> Jason.decode!()
+
         # record the decision
         Excision.Excisions.create_decision(%{
           decision_site_id: decision_site.id,
@@ -239,6 +239,6 @@ defmodule ExcisionWeb.ClassifierController do
         nil
     end
 
-    resp
+    conn
   end
 end
diff --git a/priv/static/assets/app.css b/priv/static/assets/app.css
@@ -1377,12 +1377,6 @@ select {
   margin-left: calc(0.5rem * calc(1 - var(--tw-space-x-reverse)));
 }
 
-.space-y-8 > :not([hidden]) ~ :not([hidden]) {
-  --tw-space-y-reverse: 0;
-  margin-top: calc(2rem * calc(1 - var(--tw-space-y-reverse)));
-  margin-bottom: calc(2rem * var(--tw-space-y-reverse));
-}
-
 .space-y-1 > :not([hidden]) ~ :not([hidden]) {
   --tw-space-y-reverse: 0;
   margin-top: calc(0.25rem * calc(1 - var(--tw-space-y-reverse)));
@@ -1395,6 +1389,12 @@ select {
   margin-bottom: calc(1rem * var(--tw-space-y-reverse));
 }
 
+.space-y-8 > :not([hidden]) ~ :not([hidden]) {
+  --tw-space-y-reverse: 0;
+  margin-top: calc(2rem * calc(1 - var(--tw-space-y-reverse)));
+  margin-bottom: calc(2rem * var(--tw-space-y-reverse));
+}
+
 .divide-y > :not([hidden]) ~ :not([hidden]) {
   --tw-divide-y-reverse: 0;
   border-top-width: calc(1px * calc(1 - var(--tw-divide-y-reverse)));
@@ -1692,14 +1692,14 @@ select {
   padding-bottom: 1rem;
 }
 
-.pr-6 {
-  padding-right: 1.5rem;
-}
-
 .pl-5 {
   padding-left: 1.25rem;
 }
 
+.pr-6 {
+  padding-right: 1.5rem;
+}
+
 .text-left {
   text-align: left;
 }

diff --git a/test/excision_web/controllers/decision_site_controller_test.exs b/test/excision_web/controllers/decision_site_controller_test.exs
@@ -124,6 +124,43 @@ defmodule ExcisionWeb.DecisionSiteControllerTest do
       assert %Excision.Excisions.Decision{} = decision
       assert Jason.decode!(decision.input) == Jason.decode!(Jason.encode!(messages))
     end
+
+    test "returns meaningful error when trailing slash is appended to path", %{
+      conn: conn,
+      decision_site: decision_site,
+      bypass: bypass
+    } do
+      Bypass.expect_once(bypass, "POST", "/v1/chat/completions/", fn conn ->
+        Plug.Conn.resp(
+          conn,
+          404,
+          """
+          {
+              "error": {
+                  "message": "Invalid URL (POST /v1/chat/completions/)",
+                  "type": "invalid_request_error",
+                  "param": null,
+                  "code": null
+              }
+          }
+          """
+        )
+      end)
+
+      conn =
+        conn
+        |> assign(
+          :raw_body,
+          Jason.encode!(%{
+            messages: [%{role: "user", content: "some message"}],
+            model: ""
+          })
+        )
+        |> Plug.Conn.put_req_header("authorization", "Bearer foo")
+        |> post(~p"/api/decision_sites/#{decision_site}/invoke/")
+
+      assert response(conn, 404)
+    end
   end
 
   defp create_decision_site(_) do