Hmm. This is the same signature as in the ServerFinalizer, but I'm not sure it actually makes sense here. In the server we're guaranteed to have an http.Request, and we're guaranteed to have a code, so they make sense as parameters. But in the client, we may not make it all the way to generating an http.Request, and we may not get a status code back. And, we have an explicit error value that will always be returned via the endpoint (though it may be nil). So I think the better signature here is

type ClientFinalizerFunc func(ctx context.Context, err error) 

Of course, with a note in the comment about which fields in the context will be populated under which conditions. What do you think?

makes sense to me. the main reason i went with the signature of the ServerFinalizer was from a lack of how to tackle the problem. But it makes sense to me to have a signature like that. I'll update the PR later this evening and push up the requested changes. Thanks for the comments 😸 !

I'm not sure it's correct to create zero-value http.Requests and Responses; my intuition is that they need to be created by package http in an e.g. NewReqeust constructor. But that's fine, I think we can get by without them. Going with the change in the ClientFinalizerFunc below, perhaps this can look like..

var (
    resp *http.Response
    err  error
)
if c.finalizer != nil {
    defer func() {
        if resp != nil {
            ctx = context.WithValue(ctx, ContextKeyResponseHeaders, resp.Header)
            ctx = context.WithValue(ctx, ContextKeyResponseSize, resp.ContentLength)
        }
        c.finalizer(ctx, err)
    }()
}

What do you think?

It's ridiculous how my stream-of-consciousness garbage thoughts when I was writing this test get propagated into the infinite future 🤙