[1/15] Route error responses back through the interceptor chain

core/src/main/kotlin/com/lightningkite/lightningserver/http/HttpInterceptor.kt

@@ -1,7 +1,9 @@
 package com.lightningkite.lightningserver.http
 
+import com.lightningkite.lightningserver.pathing.PathSpec
 import com.lightningkite.lightningserver.runtime.ServerRuntime
 import com.lightningkite.lightningserver.runtime.instrument
+import kotlinx.coroutines.CancellationException
 
 /**
  * Interface for intercepting and modifying HTTP requests and responses.
@@ -72,10 +74,26 @@ public fun interface HttpInterceptor {
 }
 
 /**
- * Wraps the intercept call with instrumentation for performance monitoring.
+ * Wraps the intercept call with instrumentation for performance monitoring, and recovers from
+ * exceptions thrown by this interceptor (or anything nested inside it) by converting them to a
+ * response via the configured exception handler.
  *
  * This is used internally to track the time spent in each interceptor.
  *
+ * ## Why recover here
+ * Every interceptor in the chain is composed via nested calls to this function (see
+ * [compileAndInstrument]), so recovering at this single point means an exception thrown by *any*
+ * interceptor - not just the terminal handler - is turned into a response before it unwinds past
+ * the interceptors that wrap it. Those outer interceptors then see a normal return value from
+ * their own continuation call and still get to post-process it (e.g. CORS still adds
+ * `Access-Control-Allow-Origin` to a response produced by a rate limiter's own thrown exception).
+ * Without this, only exceptions from the innermost handler were guaranteed interceptor
+ * post-processing; an interceptor throwing directly (as [com.lightningkite.lightningserver.cors.CorsInterceptor]
+ * and rate limiters do) would still skip every interceptor wrapping it.
+ *
+ * [CancellationException] is rethrown unchanged - it signals coroutine cancellation (e.g. a client
+ * disconnect), not a request-level failure, and must not be swallowed into a fabricated response.
+ *
  * @param request The HTTP request to intercept
  * @param action The continuation function
  * @return The HTTP response
@@ -85,8 +103,19 @@ public suspend inline fun HttpInterceptor.interceptInstrumented(
     request: HttpRequest<*>,
     noinline action: suspend ServerRuntime.(HttpRequest<*>) -> HttpResponse,
 ): HttpResponse {
-    return instrument(name) {
-        intercept(request, action)
+    return try {
+        instrument(name) {
+            intercept(request, action)
+        }
+    } catch (e: CancellationException) {
+        throw e
+    } catch (e: Exception) {
+        try {
+            @Suppress("UNCHECKED_CAST")
+            server.server.exceptionHandler.handle(request as HttpRequest<PathSpec>, e)
+        } catch (_: Exception) {
+            HttpResponse(status = HttpStatus.InternalServerError)
+        }
     }
 }
 

core/src/main/kotlin/com/lightningkite/lightningserver/runtime/implementationHelpers.kt

@@ -60,9 +60,28 @@ private val errorType = TelemetryKey.OfString("error.type")
  */
 public suspend fun ServerRuntime.handle(request: HttpRequest<PathSpec>): HttpResponse = instrumentHttpRequest(request) {
     var errorType: String? = null
+
+    // Turns a thrown error into an HTTP response via the configured exception handler,
+    // recording the error type for instrumentation; falls back to a bare 500 if the handler
+    // itself throws. Applied INSIDE the interceptor chain (below) so error responses still
+    // pass back through the interceptors — most importantly CORS. Otherwise error responses
+    // ship without CORS headers and browsers misreport every 4xx/5xx as a CORS failure.
+    suspend fun handleError(e: Exception, label: String? = e::class.simpleName): HttpResponse {
+        errorType = label
+        return try {
+            instrument("exceptionHandler") { server.exceptionHandler.handle(request, e) }
+        } catch (_: Exception) {
+            errorType = "unhandled_exception"
+            HttpResponse(status = HttpStatus.InternalServerError)
+        }
+    }
+
     val response = try {
         server.compiledHttpInterceptors.intercept(request) { req ->
             this.logger.info { "${request.path} accessed by ${request.sourceIp}" }
+            // Map handler/route/compression exceptions to responses in-place so the surrounding
+            // interceptors (CORS, etc.) still post-process error responses.
+            try {
             val result = try {
                 // Route resolution must live inside this try so that a RouteNotFoundException (e.g. a HEAD
                 // request with no HEAD handler, or a missing trailing slash) is caught below and recovered
@@ -173,37 +192,33 @@ public suspend fun ServerRuntime.handle(request: HttpRequest<PathSpec>): HttpRes
                 } else result.headers,
                 body = TypedData(newData, result.body.mediaType)
             )
-        }
-    } catch (timeout: TimeoutCancellationException) {
-        // A handler exceeded its HttpHandler.timeout. Map to 408 through the normal exception handler so the
-        // error body is formatted consistently. (Other CancellationExceptions — e.g. client disconnect — are
-        // NOT caught here and fall through unchanged.)
-        errorType = "timeout"
-        this.logger.warn { "Request to ${request.path} exceeded its handler timeout." }
-        instrument("exceptionHandler") {
-            server.exceptionHandler.handle(
-                request,
-                HttpStatusException(
-                    status = HttpStatus.RequestTimeout,
-                    detail = "timeout",
-                    message = "The request handler exceeded its timeout.",
-                ),
-            )
-        }
-    } catch (e: Exception) {
-        errorType = e::class.simpleName
-        try {
-            this.logger.error(e) { "Exception in HTTP" }
-            instrument("exceptionHandler") {
-                server.exceptionHandler.handle(
-                    request,
-                    e
+            } catch (timeout: TimeoutCancellationException) {
+                // A handler exceeded its HttpHandler.timeout. Map to 408 through the normal exception handler so
+                // the error body is formatted consistently. (Other CancellationExceptions — e.g. client
+                // disconnect — are handled by the generic catch below, matching prior behavior.)
+                this.logger.warn { "Request to ${request.path} exceeded its handler timeout." }
+                handleError(
+                    HttpStatusException(
+                        status = HttpStatus.RequestTimeout,
+                        detail = "timeout",
+                        message = "The request handler exceeded its timeout.",
+                    ),
+                    label = "timeout",
                 )
+            } catch (e: Exception) {
+                this.logger.error(e) { "Exception in HTTP" }
+                handleError(e)
             }
-        } catch (_: Exception) {
-            errorType = "unhandled_exception"
-            HttpResponse(status = HttpStatus.InternalServerError)
         }
+    } catch (e: Exception) {
+        // Last-resort safety net. Exceptions thrown by an interceptor itself are now recovered
+        // inside HttpInterceptor.interceptInstrumented, at the point each interceptor is invoked,
+        // so outer interceptors (e.g. CORS) still get to post-process the resulting response. This
+        // catch only fires when there are no interceptors installed (compiledHttpInterceptors is
+        // HttpInterceptor.NoOp, which bypasses interceptInstrumented) or some other exception
+        // escapes the chain machinery itself — headers from would-be interceptors are absent here.
+        this.logger.error(e) { "Exception in HTTP interceptor chain" }
+        handleError(e)
     }
     HttpInstrumentationResult(response, response.status.code, errorType)
 }

core/src/test/kotlin/com/lightningkite/lightningserver/runtime/ImplementationHelpersHandleTest.kt

@@ -1,6 +1,8 @@
 package com.lightningkite.lightningserver.runtime
 
 import com.lightningkite.lightningserver.HttpMethod
+import com.lightningkite.lightningserver.HttpStatusException
+import com.lightningkite.lightningserver.NotFoundException
 import com.lightningkite.lightningserver.definition.builder.ServerBuilder
 import com.lightningkite.lightningserver.definition.loggingSettings
 import com.lightningkite.lightningserver.http.*
@@ -88,6 +90,11 @@ class ImplementationHelpersHandleTest {
             HttpResponse.plainText("quick")
         }
 
+        // Always throws, to prove error responses still receive interceptor post-processing.
+        val boom = path.path("boom").get bind HttpHandler<PathSpec0> {
+            throw NotFoundException(detail = "boom", message = "Boom.")
+        }
+
         init {
             registerBasicMediaTypeCoders()
         }
@@ -390,6 +397,84 @@ class ImplementationHelpersHandleTest {
         }
     }
 
+    @Test
+    fun error_response_still_receives_cors_headers() {
+        // Regression: a handler that throws must still get CORS headers. The exception is now
+        // mapped to a response INSIDE the interceptor chain, so CORS post-processes it. Without
+        // this, the browser masks every 4xx/5xx as a CORS failure and the real error (here, a
+        // 404) is invisible to client JS.
+        TestServer.test(settings = {}) {
+            runBlocking {
+                val resp = serverRuntime.handle(
+                    HttpRequest<PathSpec>(
+                        path = RawHttpEndpoint(asString = "/boom", method = HttpMethod.GET),
+                        queryParameters = QueryParameters.EMPTY,
+                        headers = HttpHeaders { add(HttpHeader.Origin, "https://example.com") },
+                        domain = "example.com",
+                        protocol = "https",
+                        sourceIp = "local",
+                    )
+                )
+                assertEquals(HttpStatus.NotFound, resp.status)
+                assertEquals(
+                    "https://example.com",
+                    resp.headers[HttpHeader.AccessControlAllowOrigin]?.root,
+                    "error responses must carry the CORS allow-origin header",
+                )
+            }
+        }
+    }
+
+    // A minimal server whose second (innermost) interceptor always throws before calling its
+    // continuation - simulating a rate limiter or auth interceptor rejecting a request. CORS is
+    // installed first (outermost) so this proves outer interceptors still post-process a response
+    // that resulted from an *interceptor's own* thrown exception, not just a handler's.
+    object InterceptorFailureTestServer : ServerBuilder() {
+        val cors = com.lightningkite.lightningserver.cors.CorsSettings(
+            limitToDomains = listOf("example.com"),
+            limitToMethods = listOf("*"),
+        )
+
+        init {
+            install(com.lightningkite.lightningserver.cors.CorsInterceptor(setting("cors", cors)))
+            install(HttpInterceptor { _, _ ->
+                throw HttpStatusException(
+                    status = HttpStatus.TooManyRequests,
+                    detail = "boom-interceptor",
+                    message = "Simulated interceptor failure.",
+                )
+            })
+            registerBasicMediaTypeCoders()
+        }
+    }
+
+    @Test
+    fun error_thrown_by_interceptor_itself_still_receives_outer_post_processing() {
+        // Regression: an interceptor that throws directly (not the handler) must still be
+        // recovered close enough to the throw site that interceptors wrapping it - here, CORS -
+        // see a normal response back from their continuation and still post-process it.
+        InterceptorFailureTestServer.test(settings = {}) {
+            runBlocking {
+                val resp = serverRuntime.handle(
+                    HttpRequest<PathSpec>(
+                        path = RawHttpEndpoint(asString = "/anything", method = HttpMethod.GET),
+                        queryParameters = QueryParameters.EMPTY,
+                        headers = HttpHeaders { add(HttpHeader.Origin, "https://example.com") },
+                        domain = "example.com",
+                        protocol = "https",
+                        sourceIp = "local",
+                    )
+                )
+                assertEquals(HttpStatus.TooManyRequests, resp.status)
+                assertEquals(
+                    "https://example.com",
+                    resp.headers[HttpHeader.AccessControlAllowOrigin]?.root,
+                    "CORS (an outer interceptor) must still post-process a response produced by an inner interceptor's own thrown exception",
+                )
+            }
+        }
+    }
+
     @Test
     fun fast_handler_completes_within_its_timeout() {
         TestServer.test(settings = {}) {