Let response return a HTTP.Response object

[schlichtanders]

I am currently writing redirect statements with Mux.jl which is made very difficult, because standard HTTP.Response objects are not supported directly as a response

• response does not create a HTTP.Response, but a mere Dict. It seems there is entirely no need for this extra Layer, because HTTP.Response is a mutable object
• toresponse will then turn the Dict into a HTTP.Response, however does not handle HTTP.Response objects as input as far as I can see

I want to suggest to completely deleting this extra Dict layer (including deleting toresponse) and directly return HTTP.Response objects instead

[cmcaine]

Hello. Do you remember why the response dicts were making it hard for you to do redirects?

In any case, Mux is kinda just a system for making your own middleware system in. You can easily replace the toresponse middleware with something of your own or add your own middleware after toresponse that will mutate the HTTP.Response value that it generates.

my_defaults = stack(todict, basiccatch, splitquery, my_toresponse)
@app myapp = (
    my_defaults,
    page("/about", respond("hi")),
    Mux.notfound())

[schlichtanders]

I am still using Mux, but have adapted a lot to admit.

I think the main reason I was confused is that Mux execution is still rather intransparent for me. My first initial guess was that all the mux Middleware is everything you need to know. Hence directly returning https response objects didn't seem possible.

Turns out that there is hard coded special support for directly returning https.response objects. It makes everything look much more complicated than it need to be from my perspective.

So I guess having read more of mux source code by now, I would rather argue to get rid of all the special hard coded handling and do everything with Middleware. (It would also be great to massively improve the error message, but that is another issue)

[cmcaine]

Thanks for sharing!

I have a PR to make the error messages better here #157 which I guess I'll merge now cos it has been a few days.

I'm not sure what you mean by hardcoding? Could you explain with examples?

The intention is that Mux only interfaces with the HTTP.Request object in todict and the HTTP.Response object in toresponse and mk_response. And regular users aren't intended to use any of those unless the defaults don't work for them.

The rest of the built-in middleware is supposed to accept dicts in one format (with keys :uri, :path, etc) and return dicts in another (with keys :status, :body, :headers). Users are intended to add their own stuff to the dicts in middleware if they need to.

The main bit of coupling that I know of that is more publicly visible is that we do some type piracy on HTTP.Response (eek!) and the keyword args to Mux.serve() are specific to HTTP.jl

[schlichtanders]

This is the main hardcoded part if I remember it correctly (mk_response)
https://github.com/JuliaWeb/Mux.jl/blob/master/src/server.jl#L32-L50

# conversion functions for known http_handler return objects
mk_response(d) = d
function mk_response(d::Dict)
  r = HTTP.Response(get(d, :status, 200))
  haskey(d, :body) && (r.body = d[:body])
  haskey(d, :headers) && (r.headers = d[:headers])
  return r
end

function http_handler(app::App)
  handler = (req) -> mk_response(app.warez(req))
  # handler.events["error"]  = (client, error) -> println(error)
  # handler.events["listen"] = (port)          -> println("Listening on $port...")
  return handler
end

function ws_handler(app::App)
  handler = (sock) -> mk_response(app.warez(sock))
  return handler
end

My inner intuition expects this to be configurable, as part of the standard default middlewares. But it is not - it is not replaceable, configurable, but has a special extra place and will always be called (not sure about the overhead, hopefully julia can strip this off completely)

For me it would be so much more intuitive to have

ws_handler(app::App) = app.warez
http_handler(app::App) = app.warez

instead, bringing the transformation to HTTP.Response object as a simple middleware.

[cmcaine]

mk_response will pass HTTP.Response objects through unchanged (see its first method), so it won't stop you if you choose to replace the default toresponse middleware with your own.

If the middleware toresponse gets an HTTP.Response object it will either throw an error or mangle the object because it will try to convert it to a string like this:

Mux.jl/src/basics.jl

Line 44 in 1d04884

Response(o) = HTTP.Response(stringmime(MIME"text/html"(), o))

You can avoid that by providing your own toresponse method (toresponse(res::HTTP.Response) = res) or by writing your own middleware.

Knowing this, would you still like any changes to Mux? Maybe documenting what to do if you want to accept or return HTTP.Request/Response objects directly? Any code changes?

[schlichtanders]

I know everything what you mentioned and have changed the toresponse method respectively for my case.

I feel a bit like repeating myself: The code change I would suggest is to get all transformations into middlewares so that really

ws_handler(app::App) = app.warez
http_handler(app::App) = app.warez

is the only thing left "hard-coded"

It makes the entire design of Mux much clearer and hence also easier to understand for newcomers.