diff --git a/Sources/App/Core/ErrorMiddleware.swift b/Sources/App/Core/ErrorMiddleware.swift index f78dc68c1..a89b1665d 100644 --- a/Sources/App/Core/ErrorMiddleware.swift +++ b/Sources/App/Core/ErrorMiddleware.swift @@ -25,9 +25,14 @@ final class ErrorMiddleware: AsyncMiddleware { func respond(to req: Request, chainingTo next: AsyncResponder) async throws -> Response { do { return try await next.respond(to: req) - } catch let error as AbortError where error.status.code >= 400 { - let statusCode = error.status.code - let isCritical = (statusCode >= 500) + } catch let error as AbortError where error.status.code < 400 { + throw error + } catch { + // Errors that aren't `AbortError`s would otherwise escape this middleware and be + // turned into an unlogged, raw 500 by Vapor's default error handling. + // See https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/3944 + let abortError = error as? AbortError ?? Abort(.internalServerError) + let isCritical = (abortError.status.code >= 500) @Dependency(\.logger) var logger @@ -37,9 +42,9 @@ final class ErrorMiddleware: AsyncMiddleware { logger.error("\(error): \(req.url)") } - return ErrorPage.View(path: req.url.path, error: error) + return ErrorPage.View(path: req.url.path, error: abortError) .document() - .encodeResponse(status: error.status) + .encodeResponse(status: abortError.status) } } diff --git a/Tests/AppTests/ErrorMiddlewareTests.swift b/Tests/AppTests/ErrorMiddlewareTests.swift index d13c2b2e1..338b015c1 100644 --- a/Tests/AppTests/ErrorMiddlewareTests.swift +++ b/Tests/AppTests/ErrorMiddlewareTests.swift @@ -26,8 +26,11 @@ extension AllTests.ErrorMiddlewareTests { app.get("ok") { _ in return "ok" } app.get("404") { req async throws -> Response in throw Abort(.notFound) } app.get("500") { req async throws -> Response in throw Abort(.internalServerError) } + app.get("unknown-error") { req async throws -> Response in throw TestError() } } + struct TestError: Error { } + @Test func custom_routes() async throws { try await withSPIApp(setup) { app in // Test to ensure the test routes we've set up in setUpWithError are in effect @@ -72,4 +75,20 @@ extension AllTests.ErrorMiddlewareTests { } } + @Test func non_abort_error() async throws { + // Ensure errors that aren't AbortErrors are also converted to html error pages + // https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/3944 + try await withDependencies { + $0.environment.dbId = { nil } + } operation: { + try await withSPIApp(setup) { app in + try await app.testing().test(.GET, "unknown-error", afterResponse: { response async in + #expect(response.status == .internalServerError) + #expect(response.content.contentType == .html) + #expect(response.body.asString().contains("500 - Internal Server Error")) + }) + } + } + } + }