From 4ef4f4fbf655ad8c350d456522d02208c80fd639 Mon Sep 17 00:00:00 2001
From: Graham McIntire <graham@mcintire.me>
Date: Wed, 4 Feb 2026 12:18:14 -0600
Subject: [PATCH] handle exceptions gracefully on api endpoints

---
 docs/exception_handling.md                 | 210 +++++++++++++++++++++
 lib/towerops_web/plug_exceptions.ex        |  69 +++++++
 test/towerops_web/plug_exceptions_test.exs |  56 ++++++
 3 files changed, 335 insertions(+)
 create mode 100644 docs/exception_handling.md
 create mode 100644 lib/towerops_web/plug_exceptions.ex
 create mode 100644 test/towerops_web/plug_exceptions_test.exs

diff --git a/docs/exception_handling.md b/docs/exception_handling.md
new file mode 100644
index 00000000..95ebf581
--- /dev/null
+++ b/docs/exception_handling.md
@@ -0,0 +1,210 @@
+# Exception Handling Strategy
+
+## Overview
+
+Towerops implements the `Plug.Exception` protocol for expected client errors to reduce noise in error tracking systems and provide clean HTTP responses for invalid requests.
+
+## Problem Statement
+
+Production Phoenix applications receive many "exceptions" that aren't actually application failures:
+
+1. **API clients** sending incorrect `Accept` headers (triggers `Phoenix.NotAcceptableError`)
+2. **Mobile apps** with stale CSRF tokens after backgrounding (triggers `InvalidCSRFTokenError`)
+3. **Automation tools** (Terraform, scripts) with malformed requests
+
+These are legitimate **client errors (400-level)**, not server failures (500-level). Without proper handling:
+
+- Exception tracking services (Sentry, Rollbar, etc.) send alerts for every bad request
+- Logs are cluttered with stack traces for expected behavior
+- Difficult to identify genuine application bugs in the noise
+
+## Solution
+
+Implement `Plug.Exception` protocol for expected exceptions to convert them into clean HTTP 400 responses.
+
+### Implementation
+
+Located in: `lib/towerops_web/plug_exceptions.ex`
+
+```elixir
+defimpl Plug.Exception, for: Phoenix.NotAcceptableError do
+  def status(_exception), do: 400
+  def actions(_exception), do: []
+end
+
+defimpl Plug.Exception, for: Plug.CSRFProtection.InvalidCSRFTokenError do
+  def status(_exception), do: 400
+  def actions(_exception), do: []
+end
+```
+
+### How It Works
+
+1. **Before**: Exception raised → 500 error → stack trace logged → alert sent
+2. **After**: Exception raised → protocol converts to 400 → debug log only → no alert
+
+Phoenix checks if an exception implements `Plug.Exception` protocol:
+- If yes: calls `status/1` to get HTTP status code, returns that status
+- If no: treats as 500 Internal Server Error, logs stack trace
+
+## Covered Exception Types
+
+### Phoenix.NotAcceptableError
+
+**Triggered when**: Client sends `Accept` header that doesn't match controller format.
+
+**Common scenarios**:
+- API client requesting `Accept: application/xml` when only JSON is supported
+- Automation tools with default headers hitting JSON endpoints
+- Browser requests to API-only endpoints
+
+**Example**:
+```bash
+curl -H "Accept: application/xml" https://towerops.net/api/v1/sites
+# Returns: 400 Bad Request (not 500)
+```
+
+### Plug.CSRFProtection.InvalidCSRFTokenError
+
+**Triggered when**: CSRF token is missing, invalid, or expired.
+
+**Common scenarios**:
+- Mobile app resumed after hours of inactivity (token expired)
+- User left browser tab open overnight (session invalidated)
+- Legitimate request replayed after logout/login cycle
+
+**Example**:
+- User opens form, leaves for lunch, returns and submits → 400 (not 500)
+- iOS app backgrounded for hours, user returns and taps button → 400
+
+## Monitoring & Security
+
+### Debug Logging
+
+Both exception types log at debug level before converting:
+
+```elixir
+Logger.debug("Converting Phoenix.NotAcceptableError to 400: #{inspect(exception)}")
+```
+
+This allows:
+- Visibility in development (`mix phx.server` shows debug logs)
+- Optional monitoring in production (enable debug logs temporarily)
+- No production alerts for expected behavior
+
+### Security Considerations
+
+**CSRF Protection Still Active**: Converting to 400 doesn't disable CSRF protection:
+- Invalid tokens still rejected (request fails)
+- User sees "Bad Request" instead of generic error page
+- Audit logs still capture failed requests
+
+**Attack Detection**: Excessive CSRF errors may indicate:
+- Automated CSRF attack attempts
+- Client implementation bugs
+- Session fixation attempts
+
+**Recommended**: Set up monitoring for 400 error rate spikes on protected endpoints.
+
+### What We're NOT Doing
+
+This pattern should **only** apply to expected client errors:
+
+❌ **Don't convert**:
+- Database connection errors (500)
+- Ecto query errors (500)
+- Permission violations (403, handled separately)
+- Authentication failures (401, handled separately)
+- Not found errors (404, handled separately)
+
+✅ **Do convert**:
+- Malformed request format (400)
+- Invalid content negotiation (400)
+- Stale CSRF tokens (400)
+
+## Testing
+
+Tests verify both the protocol implementation and actual HTTP behavior:
+
+```elixir
+# Protocol implementation
+test "implements status/1 callback" do
+  exception = %Phoenix.NotAcceptableError{}
+  assert Plug.Exception.status(exception) == 400
+end
+
+# Actual HTTP behavior
+test "returns 400 for invalid Accept header" do
+  conn =
+    build_conn()
+    |> put_req_header("accept", "application/invalid")
+    |> get("/api/v1/sites")
+
+  assert conn.status == 400
+end
+```
+
+See: `test/towerops_web/plug_exceptions_test.exs`
+
+## Adding New Exception Types
+
+When adding new exception types, follow this checklist:
+
+1. **Verify it's a client error**:
+   - Is this caused by invalid user input or client behavior?
+   - Would a well-behaved client avoid this error?
+   - Is this a 400-level status code scenario?
+
+2. **Add protocol implementation** in `plug_exceptions.ex`:
+   ```elixir
+   defimpl Plug.Exception, for: YourException do
+     def status(_exception), do: 400
+     def actions(_exception), do: []
+   end
+   ```
+
+3. **Add tests** in `plug_exceptions_test.exs`:
+   - Test protocol callbacks (`status/1`, `actions/1`)
+   - Test actual HTTP behavior (integration test)
+
+4. **Update documentation**:
+   - Add to "Covered Exception Types" section above
+   - Document common scenarios that trigger it
+   - Add example request/response
+
+5. **Monitor in production**:
+   - Watch error rates for first week after deployment
+   - Verify no legitimate errors are being masked
+   - Adjust if needed
+
+## Production Rollout
+
+When deploying this pattern:
+
+1. **Before deployment**:
+   - Review exception tracking service (Sentry, Rollbar, etc.)
+   - Note baseline error rates for each exception type
+   - Set up 400 status code monitoring (separate from alerts)
+
+2. **After deployment**:
+   - Verify exception alerts decreased for covered types
+   - Check 400 error rates in metrics (should replace 500s)
+   - Spot-check logs to ensure legitimate errors still visible
+
+3. **Long-term**:
+   - Review 400 error patterns quarterly
+   - Consider adding new exception types if patterns emerge
+   - Remove if causing issues (easily reversible)
+
+## References
+
+- **Original Article**: [Handle Phoenix Exceptions Gracefully](https://peterullrich.com/handle-phoenix-exceptions-gracefully)
+- **Plug.Exception Protocol**: https://hexdocs.pm/plug/Plug.Exception.html
+- **Phoenix Error Handling**: https://hexdocs.pm/phoenix/errors.html
+
+## Maintenance Log
+
+| Date | Change | Reason |
+|------|--------|--------|
+| 2026-02-04 | Initial implementation | Reduce error tracking noise from API clients and stale CSRF tokens |
+
diff --git a/lib/towerops_web/plug_exceptions.ex b/lib/towerops_web/plug_exceptions.ex
new file mode 100644
index 00000000..8b4a5668
--- /dev/null
+++ b/lib/towerops_web/plug_exceptions.ex
@@ -0,0 +1,69 @@
+defmodule ToweropsWeb.PlugExceptions do
+  @moduledoc """
+  Protocol implementations for converting expected Phoenix exceptions
+  into clean HTTP status codes rather than raising errors.
+
+  This reduces noise in error tracking by treating invalid requests
+  (wrong MIME types, invalid CSRF tokens) as bad requests rather than
+  application failures.
+
+  ## Rationale
+
+  Production applications receive noise from legitimate bad requests:
+  - API clients sending incorrect Accept headers
+  - Mobile apps with stale CSRF tokens after backgrounding
+  - Automation tools (Terraform, scripts) with malformed requests
+
+  These are client errors (400-level), not server failures (500-level).
+  By implementing `Plug.Exception` protocol for these exceptions, we:
+
+  1. Return clean HTTP 400 Bad Request responses
+  2. Reduce exception tracking alerts for expected client errors
+  3. Keep error logs focused on genuine application failures
+
+  ## Implementation
+
+  Each exception type gets a protocol implementation with two callbacks:
+
+  - `status/1` - Returns the HTTP status code (400 for bad requests)
+  - `actions/1` - Returns suggested actions (empty list for now)
+
+  ## Covered Exception Types
+
+  - `Phoenix.NotAcceptableError` - Invalid Accept headers from API clients
+  - `Plug.CSRFProtection.InvalidCSRFTokenError` - Stale or invalid CSRF tokens
+
+  ## Monitoring
+
+  These exceptions are still logged at debug level, allowing monitoring
+  without triggering production alerts. If you see patterns indicating
+  attacks (e.g., excessive CSRF failures), investigate via logs.
+
+  ## See Also
+
+  - Documentation: `docs/exception_handling.md`
+  - Tests: `test/towerops_web/plug_exceptions_test.exs`
+  """
+
+  require Logger
+
+  # Invalid Accept headers from API consumers (automation, mobile apps, etc.)
+  defimpl Plug.Exception, for: Phoenix.NotAcceptableError do
+    def status(exception) do
+      Logger.debug("Converting Phoenix.NotAcceptableError to 400: #{inspect(exception)}")
+      400
+    end
+
+    def actions(_exception), do: []
+  end
+
+  # Stale CSRF tokens from mobile app or browser sessions
+  defimpl Plug.Exception, for: Plug.CSRFProtection.InvalidCSRFTokenError do
+    def status(exception) do
+      Logger.debug("Converting InvalidCSRFTokenError to 400: #{inspect(exception)}")
+      400
+    end
+
+    def actions(_exception), do: []
+  end
+end
diff --git a/test/towerops_web/plug_exceptions_test.exs b/test/towerops_web/plug_exceptions_test.exs
new file mode 100644
index 00000000..0d54e17a
--- /dev/null
+++ b/test/towerops_web/plug_exceptions_test.exs
@@ -0,0 +1,56 @@
+defmodule ToweropsWeb.PlugExceptionsTest do
+  use ToweropsWeb.ConnCase, async: true
+
+  alias Plug.CSRFProtection.InvalidCSRFTokenError
+
+  describe "Phoenix.NotAcceptableError protocol implementation" do
+    test "implements status/1 callback" do
+      exception = %Phoenix.NotAcceptableError{}
+      status = Plug.Exception.status(exception)
+      assert status == 400
+    end
+
+    test "implements actions/1 callback" do
+      exception = %Phoenix.NotAcceptableError{}
+      actions = Plug.Exception.actions(exception)
+      assert actions == []
+    end
+
+    test "returns 400 status code when sent", %{conn: conn} do
+      # Verify that when the exception is raised and handled by Phoenix,
+      # it returns 400 (not 500) due to our protocol implementation
+      assert_error_sent 400, fn ->
+        conn
+        |> put_req_header("accept", "application/invalid")
+        |> get("/api/v1/sites")
+      end
+    end
+  end
+
+  describe "Plug.CSRFProtection.InvalidCSRFTokenError protocol implementation" do
+    test "implements status/1 callback" do
+      exception = %InvalidCSRFTokenError{}
+      status = Plug.Exception.status(exception)
+      assert status == 400
+    end
+
+    test "implements actions/1 callback" do
+      exception = %InvalidCSRFTokenError{}
+      actions = Plug.Exception.actions(exception)
+      assert actions == []
+    end
+
+    test "returns 400 status code when exception is raised" do
+      # Verify that the protocol converts the exception to 400
+      # This simulates what Phoenix does when handling the exception
+      exception = %InvalidCSRFTokenError{}
+
+      # Phoenix checks Plug.Exception protocol to determine status code
+      status = Plug.Exception.status(exception)
+      assert status == 400
+
+      # In production, this means Phoenix returns 400 instead of 500
+      # when CSRF protection raises InvalidCSRFTokenError
+    end
+  end
+end