Set env['rack.hijack'] to client.method(:full_hijack) instead of client instance

[MSP-Greg]

Description

Currently, Puma::Request sets env['rack.hijack'] to the Puma::Client instance. Instead of using the client instance, pass a method object that responds to call.

The Rack spec for the object passed in env['rack.hijack'] is that it must respond to call. Also, Puma::Client is a ':nodoc:` class, so it shouldn't be considered part of Puma's public API. Hence, there is no reason to pass it to the app.

Your checklist for this pull request

• I have reviewed the guidelines for contributing to this repository.
• I have added (or updated) appropriate tests if this PR fixes a bug or adds a feature.
• My pull request is 100 lines added/removed or less so that it can be easily reviewed.
• If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
• If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
• I have updated the documentation accordingly.
• All new and existing tests passed, including Rubocop.

[nateberkopec]

The object that we put into rack.hijack is absolutely part of our public API, so I think you have to call this a breaking change.

[nateberkopec]

Or, the client is initialized with a socket,

I'm not sure I understand this part.

[nateberkopec]

I'm 👍 for this change (though we may want to wait a few weeks to merge, let's let the next release bake a bit and see if it will be Puma 7 or not...)

[dentarg]

I see what this does (and if you only call call on this object this change shouldn't be breaking, right?) but can we spell out the benefits/motivation as to why we want to make this change?

[MSP-Greg]

@dentarg

I see what this does (and if you only call call on this object this change shouldn't be breaking, right?)

Correct. The only requirement (from the Rack spec) is that the object responds to call. Passing the Client object isn't needed, and the main 'user' items in the Client (env, req.body & and socket) are available in other ways to the user's app.

@nateberkopec

Or, the client is initialized with a socket,

I'll rephrase or just remove.

There are a few env settings that remain the same for the life of the Client object, but current code is setting them on every request, which passes thru the code in Request#handle_request. So, the 'life-time' env setting should be moved to the code in Client.initialize. That's independent of this, so it can/should be a separate PR.

---

Re the public API, Puma is much better than it used to be. I think minimal public API's are best, as they allow more flexibility with refactors, optimizations, etc. Two considerations though, are they too restrictive for the user, and do they force test code to have too many hacks. I consider things like instance_variable_get to be ok for use in test code.

EDIT: Revised the opening comment