Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is there a way to pass variables between the onUploadCreate and the onUploadFinish functions? #593

Closed
sammibajrami opened this issue Mar 26, 2024 · 12 comments

Comments

@sammibajrami
Copy link

I have figured out a way to do it but it's very awkward and also involves changing and adding a few lines in the @tus/server/dist/handlers/PostHandler.js file.

Basically the way that I have it working is that you update the "Upload-Metadata" header of the res object in the onUploadCreate function.
This header is then parsed in the PostHandler.js file and the result overwrites the upload.metadata variable, and then the upload object passes the extra metadata to the onUploadFinish function.

Any chance that there is a better way already built in that I'm missing?

Thank you in advance!

@Murderlon
Copy link
Member

Hi, this is an XY problem. What is the problem you are trying to solve? Are you only communicating between hooks or does the client also need to know this value?

Maybe you can just do it in server memory with a Map. Or if it needs to survive server restarts, put it in something like Redis?

@sammibajrami
Copy link
Author

I'm sorry, here is the problem:
I need to pass a variable from the onUploadCreate function to the onUploadFinish function. It does not need to be sent back to the client.

Is there a way to do this?

If not, then I'll look into your other suggestions, thank you!

@Murderlon
Copy link
Member

I don't know the requirements and why you are trying to pass that variable. There are many ways to write/read to different things depending on what you are doing, but those all fall into your hands and setup, there is no built-in way of doing this.

@Murderlon Murderlon closed this as not planned Won't fix, can't repro, duplicate, stale Mar 26, 2024
@sammibajrami
Copy link
Author

I'm sorry for being unclear.

I'm using the onUploadCreate function to validate the upload request, a process that generates a unique ID that I need to get in the onUploadFinish function.

Redis would be a way to do this, but I was hoping that I could simply update the metadata of the upload, but it sounds like that is not an option, correct?

@Murderlon
Copy link
Member

If you tell me why you need the ID to be accessible by the onUploadFinish hook, we can see if there are alternative approaches.

You can't change the metadata in onUploadCreate in a way that's persisted in the store too. We may want to support that at some point.

@sammibajrami
Copy link
Author

Once the upload is completed I create an entry for it, that includes the unique ID, in a separate database.

I'd be happy to share the code that I wrote to make the changes to the upload object persistent or even make a pull request if you think it'd help.
I've got it down to just adding two lines and changing two lines in the tus-node-server/packages/server/src/handlers/PostHandler.ts file.

The main issue with my approach is that the changes do not persist all the way back to the client (which isn't necessary in my case, but still).

@Murderlon
Copy link
Member

Once the upload is completed I create an entry for it, that includes the unique ID, in a separate database.

Can't you just validate the upload in onUploadCreate, reject if needed, and only create and store the ID in onUploadFinish without the need to communicate between hooks?

@sammibajrami
Copy link
Author

I would, but the ID is created as part of the validation process.
I'll just create a fork for myself with those small changes I mentioned, it should be simple to keep up to date with the main repo.

That said, would you like me to create a pull request with those changes?

The way it works is that the onUploadCreate function returns both the res object and the upload object to the PostHandler which then overwrites the upload object in that file same as the res object.

I'm not sufficiently familiar with the whole codebase to be sure that that's the best way to do it, what do you think?

@sammibajrami
Copy link
Author

sammibajrami commented Mar 26, 2024

For reference, here is the updated code:

        let upload = new models_1.Upload({
            id,
            size: upload_length ? Number.parseInt(upload_length, 10) : undefined,
            offset: 0,
            metadata,
        });
        if (this.options.onUploadCreate) {
            try {
                const afterUploadCreate = await this.options.onUploadCreate(req, res, upload);
                res = afterUploadCreate[0];
                upload = afterUploadCreate[1];
            }
            catch (error) {
                log(`onUploadCreate error: ${error.body}`);
                throw error;
            }
        }

@sammibajrami
Copy link
Author

@Murderlon
Copy link
Member

Thanks for sharing. We can't allow to override the entire upload, only metadata should be allowed, and the metadata should be validated as well. It's also important to make it backwards compatible to prevent a breaking change.

I'll take next week probably. Created an issue: #594

@sammibajrami
Copy link
Author

No problem, thank you for taking the time to help me!

While I'm sure that there is a smoother way to do this, here is updated code that only overwrites upload.metadata, while also being backwards compatible:

        let upload = new models_1.Upload({
            id,
            size: upload_length ? Number.parseInt(upload_length, 10) : undefined,
            offset: 0,
            metadata,
        });
        if (this.options.onUploadCreate) {
            try {
                const afterUploadCreate = await this.options.onUploadCreate(req, res, upload);
                res = afterUploadCreate[0];
                if (afterUploadCreate[1]) {
	                upload.metadata = afterUploadCreate[1].metadata;
                }
            }
            catch (error) {
                log(`onUploadCreate error: ${error.body}`);
                throw error;
            }
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants