tus / tusd

Reference server implementation in Go of tus: the open protocol for resumable file uploads
https://tus.github.io/tusd
MIT License
2.95k stars 467 forks source link

All HookResponse fields are Ignored for Post-Create #1009

Closed jimydavis closed 9 months ago

jimydavis commented 9 months ago

I have a use case where I only create the upload row in my db AFTER the S3 upload creation (e.g. creation of the .info file) is completed. This code is inside post-create. Thereafter, if my db row writing fails, I would like the entire request to die and notify client that everything didn't go through. It's ok that I have a dangling S3 .info file as the file system is not the source of truth but my db is.

This means that if we assume tusd lives inside another app, I am not sure how this implementation can promote atomicity. S3 (or any other store for that matter) can fail and db writing can fail too. The writing to S3 only occurs after pre-create too.

My workaround: I am trying to sandwich tusd as some sort of middleware inside gin so that after the tusd handler is done, my gin context takes over and send a 400 or 500 in case my db writing inside post-create fails. But it seems impossible to be able to do this as the post-create hook does not have any access at all to the original http handler's context. So I cannot pass anything down to the next handler. I also cannot insert something like a channel as I cannot find any place where this channel can be passed into the hook or later accessed again outside of the hook.

Please correct me if I am wrong. Any suggestions highly welcome! Thank you.

PS: I have made quite a number of questions and would be keen to contribute PRs with your guidance.

Acconut commented 9 months ago

As you said correctly and as is mentioned explicitly in the docs at https://github.com/tus/tusd/blob/main/docs/hooks.md#list-of-available-hooks, post-create is non-blocking and therefore tusd does not wait for its completion before sending the response. Basically, what you are trying to do right now is not possible yet. I would recommend you to restructure your hooks implementation to create the DB entry in the pre-create hook, but mark it as preliminary. Preliminary meaning that the actual objects on S3 might not be created. But when the post-create, post-finish or post-receive hooks arrive you can start marking it as non-preliminary. Not the best solution, but something :)

I wonder how this could be improved in the future. The current design where post-create is delivered via a channel does not allow tusd to wait on the hook's completion.

jimydavis commented 9 months ago

Hey the suggestion is very helpful. In Post-Create, I can add a column called "confirmed" or something and can verify it independently from AWS directly.

jimydavis commented 9 months ago

@Acconut The other well known pattern at least I know of is from the Auth0 JWT middleware repo. There are two parts to it:

Acconut commented 9 months ago

Those are interesting concepts, thanks for sharing. When talking about hooks and events, we always have to consider two use cases: Using tusd as a package where events can be delivered via channels or function calls. And using tusd as a stand-alone program where hooks usually require contacting an external program. That makes it sometimes a bit more challenging to find a suitable approach for new hooks features :)