SaturnFramework / Saturn

Opinionated, web development framework for F# which implements the server-side, functional MVC pattern
https://saturnframework.org
MIT License
703 stars 108 forks source link

When using channels, exceptions are not propagated properly #288

Closed retendo closed 3 years ago

retendo commented 3 years ago

I encountered this issue when using channels in Saturn via add_channel in application CE.

Take the following code:

open System

open Microsoft.AspNetCore.Http
open Microsoft.Extensions.Logging
open Saturn
open Giraffe
open Saturn.Channels
open FSharp.Control.Tasks.Affine

let testController = controller {
    index (fun (ctx: HttpContext) ->
        let logger = ctx.GetLogger<String>()
        logger.Log(LogLevel.Information, "BEFORE FAIL")
        failwith "NOPE!"
        logger.Log(LogLevel.Information, "AFTER FAIL")
        "Index handler version 1" |> Controller.text ctx
    )
}

let testRouter = router {
    forward "/test" testController
}

let testChannel = channel {
    join (fun ctx info -> task { return JoinResult.Ok })
}

let app = application {
    use_router testRouter
    // commenting out the next line, gets you the expected result (500 response + stack trace in console)
    add_channel "/channel" testChannel
}

run app

Not expected If I run this and go to localhost:5000/test I get an empty 200 response:

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/test - -
info: string[0]
      BEFORE FAIL
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/1.1 GET http://localhost:5000/test - - - 200 0 - 336.7365ms

Expected If I comment out the add_channel line in application CE, I get the expected result. Exception is logged in console and the response is 500 "NOPE!":

info: Microsoft.AspNetCore.Hosting.Diagnostics[1]
      Request starting HTTP/1.1 GET http://localhost:5000/test - -
info: string[0]
      BEFORE FAIL
fail: Giraffe.Middleware.GiraffeErrorHandlerMiddleware[0]
      An unhandled exception has occurred while executing the request.
      System.Exception: NOPE!
         at Saturn.Controller.Yield@110-1.Invoke(HttpContext _arg1, Exception ex)
         at Saturn.Controller.controllerWithErrorHandler@355-4.Invoke(Exception _arg2)
         at Ply.TplPrimitives.tryWith[u](FSharpFunc`2 continuation, FSharpFunc`2 catch)
         at Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
         at Giraffe.Core.chooseHttpFunc@122.Invoke(Unit unitVar)
         at Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
         at Giraffe.Core.chooseHttpFunc@122.Invoke(Unit unitVar)
         at Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
         at Giraffe.SubRouting.routeWithPartialPath@24.Invoke(Unit unitVar)
         at Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
         at Giraffe.Core.chooseHttpFunc@122.Invoke(Unit unitVar)
         at Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
         at Giraffe.Middleware.Invoke@31.Invoke(Unit unitVar)
         at Ply.TplPrimitives.ContinuationStateMachine`1.System-Runtime-CompilerServices-IAsyncStateMachine-MoveNext()
         at Giraffe.Middleware.Invoke@64-5.Invoke(Unit unitVar)
         at Ply.TplPrimitives.tryWith[u](FSharpFunc`2 continuation, FSharpFunc`2 catch)
info: Microsoft.AspNetCore.Hosting.Diagnostics[2]
      Request finished HTTP/1.1 GET http://localhost:5000/test - - - 500 7 application/json;+charset=utf-8 470.8031ms

Any ideas? Could it be that services are added in such an order that the regular error handling is ignored, or something like that?

NinoFloris commented 3 years ago

You could try remove everything past |> on this line (channel socket middleware comes before giraffe), it seems suspect and it looks unnecessary https://github.com/SaturnFramework/Saturn/blob/aac8fb1c80ebab3ed96e7c92660512a0c9a2b688/src/Saturn/Channels.fs#L149

retendo commented 3 years ago

I'll give it a go, thanks

retendo commented 3 years ago

This fixes it, thanks @NinoFloris . I don't know if this has any other implications, though. Is there a possibility to fast-track this?