Passing the context to sync callbacks #740
Conversation
pkg/handler/context.go
Outdated
| @@ -19,8 +19,7 @@ type httpContext struct { | |||
|
|
|||
| func newContext(w http.ResponseWriter, r *http.Request) *httpContext { | |||
| return &httpContext{ | |||
| // TODO: Try to reuse the request's context in the future | |||
| Context: context.Background(), | |||
| Context: r.Context(), | |||
There was a problem hiding this comment.
This is not possible so easily as it breaks the S3Store (and possibly also gcsstore etc). The problem is that these stores run code using this context, which should also run even if the HTTP request has been aborted and thus the request context has been cancelled. The code in question is responsible for storing all of the data up to the moment that the request got stopped. And that is not possible if the context has been cancelled.
We haven't found a way so far to work around this problem but that is why the request context is not used for now. Let us know if you have any idea on how to solve this.
|
But isn't that behavior wanted. If the client cancels a patch request the underlying store should drop it, too. So the upload offset of an upload would actually be the old offset before the cancelled operation. |
No that is not what we want. If the upload offset after a PATCH request is the same as before the PATCH request, then no data was stored and the data transfer was useless. If you then try to resume, then you have to start at the previous offset again. A tus server should always try to store as much received data as possible, regardless of whether the PATCH request is completed properly or not. That is how tus implement resumable uploads. |
|
But what about the last PATCH request. If the client cancels the request, it will not get the response anymore by the application's callback. Actually the upload is completed. Can the callback be called by the client if the upload offset pointer stands at the upload length? |
That is correct. That is why the responses for the PATCH requests should not include any critical information that must not be lost. Such information should be relied through another, application-specific system, such as another RESTful endpoint. I think that is were you misunderstood the principal of using tusd. As in #741, you are sending information in the last response to the client but that is not optimal for failure tolerance.
No, the callback is only invoked once, when the upload is completed. Never a second time. |
|
Made a change that passes at least the parent context's values but decouples the contexts again: ab78498 I wanted to change the godoc of the synchronous callback methods in config.go but unsure how to phrase it correctly because it is contradictory if the callback can be used for validation but it is not guaranteed that the caller ever gets the result. |
I don't understand that. Can you elaborate? Who is the caller? The tus client? |
right. |
|
Can you explain what the problem with the documentation is then? |
the outcome of the validation is crucial to the uploading client. if the connection drops and the request is thus cancelled the client is forced to reupload the file and it will get the outcome after that second upload attempt because the callback is called just once. |
|
cool. thanks. |
In a scenario when tusd is used as library it is most probably integrated into an application having some kind of authentication context attached to the actual tus requests. To access that and to also cancel operations when the client request is cancelled it is necessary to use the request's context in tusd and to pass the context to the callback functions to access these information in the application's domain.