Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose method to process HTTP requests on HTTP::Server #15352

Open
gabriel-ss opened this issue Jan 17, 2025 · 8 comments
Open

Expose method to process HTTP requests on HTTP::Server #15352

gabriel-ss opened this issue Jan 17, 2025 · 8 comments

Comments

@gabriel-ss
Copy link
Contributor

gabriel-ss commented Jan 17, 2025

Discussion

Hey there, Crystal team!

Currently the implementation of request processing by the HTTP::Server from stdlib is tied to a socket. This is perfectly reasonable, given the nature of the protocol. However, I would like to make a case for decoupling those into separate things. By exposing the ability to process standalone request objects, the implementation would allow different computing models to be used under the same familiar interface.

I am currently working on a shard to make it easy to deploy Crystal on AWS Lambdas. The popularity of projects like the AWS Lambda Web Adapter shows that clearly there is a demand for plug-and-play solutions for running traditional HTTP frameworks on FaaS offers, even when there is a performance penalty. Given that, one of the features that I would like to add to the shard is a transparent adapter for HTTP::Server to allow a great developer experience with the least amount of overhead possible.

I made a quick draft, but currently this is only possible by reopening the stdlib and tightly coupling the implementation with its internals:

class HTTP::Server
  class RetainedResponse < Response
    property response_io

    def initialize(@response_io : IO, @version = "HTTP/1.1")
      super(@response_io, @version)
    end

    def upgrade(&block : IO ->) : Nil
      raise "Can't upgrade connection inside a lambda."
    end

    protected def write_headers
      response_io = @response_io
      response_io.write_http_headers(status_code, headers) if response_io.responds_to? :write_http_headers
      @wrote_headers = true
    end
  end

  class Response::Output < IO
    private def unbuffered_write(slice : Bytes) : Nil
      return if slice.empty?
      ensure_headers_written
      @io.write(slice)
    rescue ex : IO::Error
      unbuffered_close
      raise ClientError.new("Error while writing data to the client", ex)
    end
  end

  def process_request(request : HTTP::Request, response : RetainedResponse)
    @processor.process(request, response)
  end

  class RequestProcessor
    def process(request : HTTP::Request, response : RetainedResponse)
      response.version = request.version
      context = Context.new(request, response)

      Log.with_context do
        @handler.call(context)
      rescue ex : ClientError
        Log.debug(exception: ex.cause) { ex.message }
      rescue ex
        Log.error(exception: ex) { "Unhandled exception on HTTP::Handler" }
        unless response.closed?
          unless response.wrote_headers?
            response.respond_with_status(:internal_server_error)
          end
        end
      ensure
        response.flush
        response.output.close
      end
    end
  end
end

Which allows me to do:

server.process_request request, response

I propose the creation of an abstract HTTP::Server::Response that does all the request processing, like setting up the cookies in the header, but that does not write the headers nor the body to the IO, leaving this task to the concrete implementation. With that, it's just a matter of exposing @processor.process as a public method of the server, and then people will be able to use the HTTP::Server interface regardless of the computing model that they're using.

I believe that this wouldn't take much work since all the functionality is already implemented; it's just a matter of changing a little how it is exposed, and this change would allow people to leverage existing code and use their favorite frameworks in a bunch of new environments.

Thanks for all your work in such a great language; I really hope to see Crystal used more and more. Have a great day!

@straight-shoota
Copy link
Member

straight-shoota commented Jan 17, 2025

I don't quite understand the specific needs for this scenario. As far as I understand, HTTP::Server::RequestProcessor#process does already work with arbitrary IO objects which should enable some generic use cases unrelated to sockets.

Could you please explain what specifically is missing from the current API?

@gabriel-ss
Copy link
Contributor Author

gabriel-ss commented Jan 17, 2025

Sure!

The first point here is that, currently, the actual RequestProcessor used by the HTTP::Server object isn't exposed to the outside, since it is built internally and assigned to the @processor instance variable. This could easily be remediated with a public processor getter or a process method on the server that delegates to @processor.

The second point about the IO is related to how these serverless services operate. Usually, you provide a callback for the service, and this callback will be invoked with a JSON representation of an HTTP request and expects another JSON representation of the response. For streaming responses, an IO may be used to communicate with the runtime, but the point is that the representation of the HTTP messages are dissociated from the HTTP protocol itself.

The HTTP::Request object can easily be built from a parsed JSON payload, so no problems here. The HTTP::Server::Response, on the other hand, is an IO that automatically build an HTTP response message as the developer writes into it, so there is no way to get the body that was written as an IO clean of headers and chunking. So basically, if the user do:

  context.response.puts "Foo"
  context.response.flush
  context.response.puts "Bar"
  context.response.flush

We always get:

HTTP/1.1 200 OK
Content-Type: text/plain
Transfer-Encoding: chunked

3\r\n
Foo\r\n            #  --|
3\r\n              #    |-- No way to get an IO with only these two
Bar\r\n            #  --|
0\r\n
\r\n

The hack that I originally posted goes around that by circumventing the serialization of headers and of chunk metadata, but it does so by modifing the internals of HTTP::Server.

@straight-shoota
Copy link
Member

Ah okay. The problem is a lot clearer now, thanks!

What do you need HTTP::Server for, though? It's essentially an application server frontend for the HTTP protocol. If you don't use that protocol, I don't see much sense in using HTTP::Server. You can just instantiate an HTTP::Server::RequestProcessor and interface with that directly. Or, for this specific use case, you might even skip RequestProcessor entirely and roll your own processor because for the most part it's just dealing with the HTTP protocol.
I suppose the implementation in your example should do well on that end. And this probably fits well into user code. We might consider it for stdlib, but I suppose it might not be straightforward to extract the HTTP specifics.

And then the main concern is replacing Response::Output which an implementation that serializes to JSON instead of HTTP, and adjusting some of the aspects of Server::Response, such as #write_headers. I suppose if we move the latter to Output and make @output configurable, you should be able to pass in your own output stream implementation, based on JSON serialization.

@gabriel-ss
Copy link
Contributor Author

gabriel-ss commented Jan 18, 2025

You can indeed drop the HTTP::Server entirely and build something specific to a serverless environment, but by then you have an entirely new framework that will vendor-lock you in a serverless solution. Imagine that I have an existing application:

# Could also be a lucky application, a kemal one or any
# application in a framework built on top of HTTP::Server
app = HTTP::Server.new do |context|
  context.response.content_type = "application/json"
  context.response.puts %({"status": "ok"})
end

that I can start by doing

app.listen 3000

But, with a compatible adapter, I can also deploy it as a serverless application by changing this one line into:

handle_events with: app, using: LambdaURLAdapter

This is huge for small businesses; with the cloud providers free tier offers, deploying a low-traffic application essentially means free hosting, and when a traditional containerized application becomes cheaper, you can just change it back with no changes to your application code.

To be clear, this is not a new idea. Python, for example, has mangum, an adapter that allows any application conforming with the ASGI standard to be deployed as a serverless application independently of the used framework. The thing is, for Crystal, the standard on top of which every application and framework is built is the HTTP::Server implementation from the standard library. That's the only reason I proposed this change in the stdlib; I could go with a standalone implementation, but none of the existing code would be automatically compatible with it. By creating an intermediate abstract layer in the response, I can roll my own implementation and it would still play nice with the ecosystem.

The example I gave of the adapter is not pseudo-code; I have already sketched an implementation that allows you to deploy an existing application written with one of the many HTTP frameworks available on top of an AWS lambda. The problem is just that it relies on reopening the stdlib.

In the end, the HTTP::Server code doesn't change that frequently, so I can maintain my patched version of it in the shard, but by having an abstraction layer akin to the one provided by the ASGI standard, alternative server implementations for different use cases could be created under a unified interface: a serverless implementation, an HTTP2/HTTP3 one, all compatible with existing applications. By not having it, shards have to either reopen stdlib, or do like DUO and roll their own interface, which ends up fragmenting the ecosystem. I understand if the core team decides that the stdlib isn't the place to define it, though.

@straight-shoota
Copy link
Member

I don't get that argument that it should go without changing the code of an application. It surely requires a change whether it binds TCP ports to listen for HTTP requests or ingests JSON somehow. Technically I suppose you could offer both at the same time but that doesn't seem very useful? Anyways, even then I think it wouldn't be unreasonable to have different mechanisms for each entrypoint.

HTTP::Server is essentially just a wrapper around HTTP::Server::RequestProcessor which handles the TCP and HTTP protocols. For context, it could be feasible to completely refactor it into a generic NetworkServer (or ApplicationServer) component: #13072
I don't think it makes much sense to add a completely different mechanism into that class.

Maybe we could enhance HTTP::Server by making the non-protocol component exchangeable (e.g. expose request_processor and or its handler). That would make it easier to re-use the same handler chain between different entrypoints.

@gabriel-ss
Copy link
Contributor Author

There are 2 or 3 lines of code that you inevitably have to change, for sure. But take this node example from AWS API gateway integration docs:

export const handler = function(event, context, callback) {
    console.log('Received event:', JSON.stringify(event, null, 2));
    var res ={
        "statusCode": 200,
        "headers": {
            "Content-Type": "*/*"
        }
    };
    var greeter = 'World';
    if (event.greeter && event.greeter!=="") {
        greeter =  event.greeter;
    } else if (event.body && event.body !== "") {
        var body = JSON.parse(event.body);
        if (body.greeter && body.greeter !== "") {
            greeter = body.greeter;
        }
    } else if (event.queryStringParameters && event.queryStringParameters.greeter && event.queryStringParameters.greeter !== "") {
        greeter = event.queryStringParameters.greeter;
    } else if (event.multiValueHeaders && event.multiValueHeaders.greeter && event.multiValueHeaders.greeter != "") {
        greeter = event.multiValueHeaders.greeter.join(" and ");
    } else if (event.headers && event.headers.greeter && event.headers.greeter != "") {
        greeter = event.headers.greeter;
    } 
    
    res.body = "Hello, " + greeter + "!";
    callback(null, res);
};

To convert this code into an HTTP application would take a full rewrite. The same could be said for all the supporting code for an application: routing, compression, authentication — all of it would have to be rewritten. But with an intermediary abstraction, you can change the said 2 or 3 lines of code that bind to a socket or to another adapter, and everything else would work without further changes.

Let me give a practical example. At my current company, we needed to deploy a stateless bot for Microsoft Teams. Teams bots are built using the bot framework, essentially an abstraction layer above a traditional node HTTP server. It would receive very low traffic, but it needed to be available 24/7. From a cost perspective, it didn't make sense to deploy it as a container, so I just plugged my code into an adapter (I think it was this one, but I might be mistaken; there are multiple packages providing this functionality for Node) by writing 4 or 5 lines of code on a separate entry point and deployed it as a serverless application. Weren't the adapter available, I would have to port the bot framework logic to work on a serverless environment.

Another example: at work, we integrate with partners through REST APIs that receive a small amount of requests but have to be available for when a partner needs to trigger an operation. We used to deploy these Python APIs on AWS Fargate, which is not really cheap, and it was very wasteful. One day, we found Mangum, the adapter I mentioned before, and decided that, given it allowed us to continue to use the frameworks and libs that the entire team was already familiarized with, we should try to deploy our APIs as serverless lambdas. We replaced the line of code that called uvicorn (the Python ASGI server) with Mangum, and since then we only pay for the computations that we actually do, which is literally zero given the free tier of AWS Lambda.

There are packages that provide this functionality for many languages, and I think their popularity shows that there is a demand for it. Sorry for the huge amounts of text arguing in favor of this change; it's just that I think it would be awesome to deploy, say, an existing Kemal app as a serverless app.

You mentioned the HTTP::Server::RequestProcessor, which is also a wrapper itself around HTTP::Handler. The HTTP::Handler almost defines the needed abstraction. The intersection with HTTP::Server is just that the HTTP::Server builds an HTTP::Handler by combining blocks and middlewares but never exposes it; if there were a public getter for the built handler, this part of the problem would be 100% solved. That way, people could still build an application by passing a block to the HTTP::Server as they are used to, and this server instance could be conveniently handed to an adapter that does its own thing with the handler. HTTP::Server would act like a factory in this case.

The other point in which HTTP::Handler falls short is that the HTTP::Server::Context carries an HTTP::Server::Response object, which always writes a formatted HTTP message to the wrapped IO. By converting HTTP::Server::Response into an abstract class that leaves to the concrete implementation how to handle writes to the IO, HTTP::Handler becomes generic in relation to the parsing and serialization of the HTTP requests and responses, respectively.

With these two changes, there would be no breaking changes, and we would have a generic interface for responding to HTTP requests in the form of HTTP::Handler. Keep in mind that this does diverge from the implementation I sketched; when I wrote it, I was only trying to minimize the number of modifications to the existing implementation.

@ysbaddaden
Copy link
Contributor

The HTTP::Server::Context carries an HTTP::Server::Response object, which always writes a formatted HTTP message to the wrapped IO. By converting HTTP::Server::Response into an abstract class that leaves to the concrete implementation how to handle writes.

Sounds good.

We can already build a HTTP::Server::Context with a manually instantiated HTTP::Request untied from the HTTP protocol details, but we can't do that for the response (HTTP::Server::Response is fully tied to the HTTP protocol).

Once we have a context, we can just pass it to any HTTP::Handler middleware, and thus to any web framework 👍

Workaround: you may introduce a MyFAAS::Response that inherits from HTTP::Server::Response and make sure that @output isn't an instance of HTTP::Server::Response::Output and that's it:

class MyFAAS::Response < HTTP::Server::Response
  def initialize(@io : IO, @version = "HTTP/1.1")
    @headers = HTTP::Headers.new
    @status = :ok
    @wrote_headers = false
    @original_output = @output = @io
  end

  def upgrade(&) : Nil
    raise "BUG: can't upgrade"
  end
end

Then:

request = HTTP::Request.new(...)
io = ...
response = MyFAAS::Response.new(io)
context = HTTP::Server::Context.new(request, response)
HTTP::Handler.call(context)

@gabriel-ss
Copy link
Contributor Author

Thanks for the insights, guys! By better studying the RequestProcessor and by avoiding writes to the Response::Output, I managed to reduce the reopening to just:

class HTTP::Server
  def handler
    @processor.handler
  end

  class RequestProcessor
    getter handler
  end
end

which is a small enough modification that I am comfortable maintaining.

Sounds good.

We can already build a HTTP::Server::Context with a manually instantiated HTTP::Request untied from the HTTP protocol details, but we can't do that for the response (HTTP::Server::Response is fully tied to the HTTP protocol).

Once we have a context, we can just pass it to any HTTP::Handler middleware, and thus to any web framework 👍

If the team sees this change as an improvement, I could work on a PR sketching this abstraction. Either way, thank you so much for the help already!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants