Add request context to handler contexts

[andrewmostello]

This pull request changes the http handlers to use the context provided in the request. This change is proposed so that the handlers are able to receive values set in the context in any upstream middleware.

I ran into this issue because we set an authorization value in the context, which I'm using downstream in a custom filestore. This doesn't work if the background context is used in the handlers, as the chain of context passed from above is broken.

Thanks!

[Acconut]

Thank you very much for this PR but this patch would break a few things. The reasons for that were laid out in another PR which did a similar thing: #309 (comment)

[andrewmostello]

Ah ok. Would that issue be handled by this? https://github.com/tus/tusd/blob/master/pkg/handler/unrouted_handler.go#L619

I figured that context.Background() was explicitly used there for a long lived issue like that.

[Acconut]

Ah ok. Would that issue be handled by this?

What exactly do you mean by that? I don't understand the question.

The problem is that some stores (e.g. the S3Store) must perform network operation even after the HTTP request is canceled in order to store the transmitted data. So we cannot use the HTTP request's context there or otherwise these network operation would be immediately canceled as well.

[andrewmostello]

Apologies - I misunderstood. Would this now be an acceptable solution instead? Instead of using the request context, use context.Background, but store the request context as a context value. That can be retrieved later using the RequestContext function, and you can then get at the request context values.

[Acconut]

@andrewmostello Deep apologies for my delay. I somehow lost track of this PR!

Would this now be an acceptable solution instead? Instead of using the request context, use context.Background, but store the request context as a context value.

Frankly, I think it would be a better approach to copy the context values into a new context which does not have a deadline. This way, the design is more in sync with the typical usage of contexts in Go. I explained this in #315 (comment) and @innovate-invent started implementing it in #342. What do you think?

[andrewmostello]

@Acconut No problem - I've been using my fork for the time being.

That works for me, and I agree that it's cleaner. I'll close out this PR in favor of #342. Thanks!

Frankly, I think it would be a better approach to copy the context values into a new context which does not have a deadline. This way, the design is more in sync with the typical usage of contexts in Go. I explained this in #315 (comment) and @innovate-invent started implementing it in #342. What do you think?