Returning 406 when mustAccept fails about giraffe HOT 8 OPEN

OK, after further testing I had to revisit this and I give up trying to use mustAccept - I just cannot see how to compose it in the manner required. Instead, I wrote my own helper:

let private jsonMimeType = MediaTypeHeaderValue.Parse("application/json")

let acceptsJson: HttpHandler =
    fun (next : HttpFunc) (ctx : HttpContext) ->
        ctx.Request.GetTypedHeaders().Accept
        |> Seq.exists jsonMimeType.IsSubsetOf
        |> function
            | true -> next ctx
            | false -> RequestErrors.notAcceptable (text $"Client does not accept MIME type: {jsonMimeType}") next ctx

This can be plumbed through with the fish operator in an intuitive manner:

GET >=> acceptsJson >=> authenticated >=> routef "/something/%O" getSomethingHandler

Here's hoping I don't find any more gotchas 🙈

from giraffe.

OK, I worked it out - sorry. Something like this:

let handler: HttpHandler  =
    choose [
        mustAccept [ "application/json" ] >=>
            choose [
                subRoute "/v1" v1Handler
            ]

        RequestErrors.notAcceptable (text "Not accepted")
    ]

from giraffe.

Actually, wait, no. That means that if the resource is not found I'll get a 406 rather than 404 😢 I remain confused and would really appreciate a pointer in the right direction 🙏

from giraffe.

I landed on this helper function, which I'm not overly happy about, but don't know if there's a better way to achieve my goals:

let mustAcceptJson (inner): HttpHandler =
    let mimeType = "application/json"
    choose [
        mustAccept [ mimeType ] >=> inner
        RequestErrors.notAcceptable (text $"Client does not accept MIME type: {mimeType}")
    ]

I use it like this:

route "/user_info" >=> mustAcceptJson (authenticated >=> userInfoHandler)

Any suggestions for refinement are welcome.

from giraffe.

Hi @johnnyggalt, thanks for opening this issue and for posting the advances you're making, it could definitely be useful for other people too!

I'll try to take a look at this problem during this week.

from giraffe.

Actually, wait, no. That means that if the resource is not found I'll get a 406 rather than 404 😢 I remain confused and would really appreciate a pointer in the right direction 🙏

So you want:

1. A middleware that checks whether the request has the necessary mime type header (for example, application/json), and returns the 406 status code if it does not have (short-circuit to the error);
2. And keep returning the 404 status code if the route does not exist.

Is that it?

from giraffe.

OK, after further testing I had to revisit this and I give up trying to use mustAccept - I just cannot see how to compose it in the manner required. Instead, I wrote my own helper:

let private jsonMimeType = MediaTypeHeaderValue.Parse("application/json")

let acceptsJson: HttpHandler =
    fun (next : HttpFunc) (ctx : HttpContext) ->
        ctx.Request.GetTypedHeaders().Accept
        |> Seq.exists jsonMimeType.IsSubsetOf
        |> function
            | true -> next ctx
            | false -> RequestErrors.notAcceptable (text $"Client does not accept MIME type: {jsonMimeType}") next ctx

This can be plumbed through with the fish operator in an intuitive manner:

GET >=> acceptsJson >=> authenticated >=> routef "/something/%O" getSomethingHandler

Here's hoping I don't find any more gotchas 🙈

I think this is the best approach. For instance, you're doing something similar to what mustAccept does: https://github.com/giraffe-fsharp/Giraffe/blob/master/src/Giraffe/Core.fs#L222-L240.

from giraffe.

you're doing something similar to what mustAccept does

Right, but that's why I thought I should be able to use mustAccept in the first place, but it does not compose in the manner required. I'm curious whether people are using mustAccept because to me it seems useless because you end up stuck if you want a "standard" experience of 406 for incorrect Accept headers whilst retaining other things like 404 for undefined routes.

PS. I'm perfectly happy with the implementation of acceptsJson I came up with, so this is now more a curiosity about mustAccept than anything else.

from giraffe.