Dont discard client sockaddr

[anuragsoni]

A less invasive change would've been to stuff the sockaddr into the request's context from within opium (it'd require moving some server_connection orchestration outside rock) as seen in https://github.com/rgrinberg/opium/pull/276/commits/20b953cf8c3abe45a7a85592f2e9a1a9cc2b83c5

But the context seems like a strange place to keep something that isn't optional? 🤔

@joseferben This will most likely cause some issues within Sihl as the request types are changing.

I wish I had used opaque types for requests and responses to begin with, as that'd have resulted in minimal breakage for users...

[anuragsoni]

We'll keep the client_address in the request's context. This change shouldn't be as invasive as adding a new parameter to the request type

[joseferben]

@joseferben This will most likely cause some issues within Sihl as the request types are changing.

In Sihl we have Opium.Request.t as part of the public API, so changing the request type breaks all Sihl users. We usually give deprecating warnings in the major version before the breaking change.

I agree with the statement that the context feels like a weird place for this piece of data, but I see your point of not breaking users.

Regarding breaking Sihl, adding a new parameter to the request type could certainly be coordinated, we sort of have breaking changes in the pipeline anyway.

[anuragsoni]

@joseferben Regarding breaking Sihl, adding a new parameter to the request type could certainly be coordinated.

That is good to know. I do have a couple of breaking changes in mind that I think will be nice to have in the long run. I'd like to make all Rock/Opium types abstract (request, response, body etc). And i'm also interested in avoiding the current setup with type aliases, re-exporting of modules, types etc, as it adds to duplication between rock and opium, and I'm not convinced that it is worth it.