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

Tus: allow accessing the file in onBeforeRequest #3977

Closed
2 tasks done
Mecanik opened this issue Aug 15, 2022 · 17 comments · Fixed by #3984
Closed
2 tasks done

Tus: allow accessing the file in onBeforeRequest #3977

Mecanik opened this issue Aug 15, 2022 · 17 comments · Fixed by #3984
Labels

Comments

@Mecanik
Copy link

Mecanik commented Aug 15, 2022

Initial checklist

  • I understand this is a feature request and questions should be posted in the Community Forum
  • I searched issues and couldn’t find anything (or linked relevant results below)

Problem

For people that tend to use Bunny.net for hosting media, they offer a TUS endpoint. However, each file uploaded must have some headers set and these must be per file basis. https://docs.bunny.net/reference/tus-resumable-uploads#upload-example-javascript

I may be a total noob at this, but at the moment I cannot see a way to set these headers for multiple files.

When using Bunny.net, you first have to "create" the video file, so for this purpose there are some options:

// Option 1
onBeforeFileAdded: (currentFile, files) => { 
console.log('onBeforeFileAdded: ', currentFile);
await PrepareVideo(); // api call, get video GUID to use in authorization
},

// Option 2
onBeforeUpload: (files) => {
console.log('onBeforeUpload: ', files);
await PrepareVideo(); // api call, get video GUID to use in authorization
},

// Option 3
uppy.on('files-added', (files) => {
 console.log('Added files', files);
 await PrepareVideo(); // api call, get video GUID to use in authorization
})

Example authorization code for single file:

uppy.use(Tus, {
	endpoint: 'https://video.bunnycdn.com/tusupload',
	retryDelays: [0, 3000, 5000, 10000, 20000, 60000, 60000],
	metaFields: [ 'filetype', 'title'],
	async onBeforeRequest (req) {
		console.log('onBeforeRequest: ', req);
		const { AuthorizationSignature, AuthorizationExpire, VideoId,  LibraryId }= await GetSignature(); // api call
		req.setHeader('AuthorizationSignature', AuthorizationSignature);
		req.setHeader('AuthorizationExpire', AuthorizationExpire);
		req.setHeader('VideoId', VideoId); // the GUID returned after "preparing" (creating) the video
		req.setHeader('LibraryId', 54460); // fixed or whatever you'd like
	},
	onShouldRetry (err, retryAttempt, options, next) {
		console.log('onShouldRetry: ', err);
		if (err?.originalResponse?.getStatus() === 401) {
		  return true
		}
		return next(err)
	},
	async onAfterResponse (req, res) {
		console.log('onAfterResponse: ', res);
		if (res.getStatus() === 401) {
		  await refreshAuthToken();
		}
	},
});

Solution

Well for starters, I would like to have async/await support in onBefore* callbacks so you can await for an api response to "prepare" the video and get the GUID.

Then I would like to have the ability to set these headers for each file, as explained above.

I`m not sure how to achieve this, if you already have a solution please do share.

Alternatives

I am really not sure what alternatives there are, please do advise.

@Murderlon
Copy link
Member

Murderlon commented Aug 15, 2022

Well for starters, I would like to have async/await support in onBefore* callbacks so you can await for an api response to "prepare" the video and get the GUID.

Tus already has support for promises in onBeforeRequest, so I think you mean the Uppy callbacks. Those don't support promises, as can be seen from the docs:

⚠️ Note that this method is intended for quick synchronous checks/modifications only. If you need to do an async API call, or heavy work on a file (like compression or encryption), you should use a custom plugin instead.


Then I would like to have the ability to set these headers for each file, as explained above.

I don't think I follow, why doesn't your example with onBeforeRequest work? That one sets headers for each file, as you want?

@Mecanik
Copy link
Author

Mecanik commented Aug 15, 2022

Yes, I meant call-backs sorry. I've read the docs and saw they do not support Promises, which is why I asked if it's possible to add.

I don't think I follow, why doesn't your example with onBeforeRequest work? That one sets headers for each file, as you want?

A bit complicated because of the Bunny.net requires you to create the video file, get the id and then generate the signature. Or perhaps I just don't understand how to do it properly? Please advise.

I prepared myself 2 API endpoints, one for creating the file and one for generating the signature required.

@Murderlon
Copy link
Member

Yes, I meant call-backs sorry. I've read the docs and saw they do not support Promises, which is why I asked if it's possible to add.

Like I said, see the reference I quoted from the docs why we will not support that :)

A bit complicated because of the Bunny.net requires you to create the video file, get the id and then generate the signature. Or perhaps I just don't understand how to do it properly? Please advise.

This is not a place for support on how to use Bunny.net. Could you explain what is missing on the Uppy side, in particular, you want to add headings per file, which you do here:

async onBeforeRequest (req) {
  console.log('onBeforeRequest: ', req);
  const { AuthorizationSignature, AuthorizationExpire, VideoId,  LibraryId }= await GetSignature(); // api call
  req.setHeader('AuthorizationSignature', AuthorizationSignature);
  req.setHeader('AuthorizationExpire', AuthorizationExpire);
  req.setHeader('VideoId', VideoId); // the GUID returned after "preparing" (creating) the video
  req.setHeader('LibraryId', 54460); // fixed or whatever you'd like
},

Why is this not enough?

@Mecanik
Copy link
Author

Mecanik commented Aug 15, 2022

I will set aside the Promises for now. Regarding headers, maybe I have not explained myself properly (which I do many times).

  1. I did not ask for support on how to user Bunny.net, I know how to use it. I'm asking for support to use Bunny.net with Uppy, same as AWS and so on.
  2. The reason why onBeforeRequest is not enough for me is because I have no access to the file. As I mentioned, I need to make a request to "create" the file, which will return me the id. As you can see in the headers, I need to specify the id.

In essence, I need to make 2 requests for each file added: 1. Create video and 2. Create signature. The append the headers to each file before uploading.

I hope that clarifies the challenge I am facing.

@Murderlon
Copy link
Member

2. he reason why onBeforeRequest is not enough for me is because I have no access to the file.

Have you tried req.getUnderlyingObject() inside onBeforeRequest to see if that has something you need?

@Mecanik
Copy link
Author

Mecanik commented Aug 16, 2022

  1. he reason why onBeforeRequest is not enough for me is because I have no access to the file.

Have you tried req.getUnderlyingObject() inside onBeforeRequest to see if that has something you need?

Yeah I tried, it returns the XMLHttpRequest but I cannot see anything about the file. So far the only part which gives me file information is onBeforeUpload, but this comes with the problem of not being async and not being able to set headers per file.

Not sure what to do with this scenario.

@Mecanik
Copy link
Author

Mecanik commented Aug 16, 2022

Ok, so I've found a solution here: https://community.transloadit.com/t/uppy-tus-add-unique-headers-per-file/15048

However, this is not async so I'm not sure what happens if I make 2x API calls inside it. Need to test, but probably won't be pretty.

@Mecanik
Copy link
Author

Mecanik commented Aug 16, 2022

After testing, it's not a viable solution. onBeforeRequest seems the only solution, but there is no access to the files so back to square one.

@Murderlon
Copy link
Member

Alright, I see the problem and I think we can add the file as a second parameter.

@Murderlon Murderlon changed the title TUS Support Multiple Authorization Headers per individual file Tus: allow accessing the file in onBeforeRequest Aug 16, 2022
@Mecanik
Copy link
Author

Mecanik commented Aug 17, 2022

Unfortunately there is a weird bug going on, it seems that if I attempt to upload 1 file, it gets uploaded 3 times for some reason.

@Mecanik
Copy link
Author

Mecanik commented Aug 17, 2022

This is the chain of events that happen:

  1. My API gets called (/create) ... returns GUID
  2. My API gets called (/auth) ... returns AUTH signature
  3. Headers get set for the file
  4. Then the library tries to do a HEAD request to https://video.bunnycdn.com/tusupload/163c2bb5198443669b2d190f69b2c909 (no idea what this ID is appended) ... returns 404 (Not Found)

Tries again:

1,2,3..
4. This time the library does POST request to https://video.bunnycdn.com/tusupload .. returns 201 (Created) and returns the id below inside a location header

Tries again:

1,2,3..
4. This time the library does a PATCH request to https://video.bunnycdn.com/tusupload/715597e0462b4a0bab715d45c18d1f55 .. returns 204 (No Content)

Upon peeking into https://docs.bunny.net/reference/tus-resumable-uploads#upload-example-javascript I can see something about upload.findPreviousUploads() but I doubt this is the problem.

My idea is that the initial HEAD request to an invalid /tusupload/id is the cause.

I don't understand how to prevent this behaviour, do you have any clues?

@Mecanik
Copy link
Author

Mecanik commented Aug 17, 2022

Forgot to mention that 3 videos get "created" but only one gets actually uploaded. Per file.

@Murderlon
Copy link
Member

Everything you're describing is according to the spec of tus. The initial HEAD request is also expected when you are resuming an upload or when an previous upload was found. For further questions please refer to the community forum or in case of a bug create a new issue.

@Mecanik
Copy link
Author

Mecanik commented Aug 17, 2022

As per your comment, the docs say: "At first, the client sends a POST request to the server to initiate the upload.".

There is no POST sent from the client, instead a direct HEAD request is sent.

I don't understand what the heck are with your sarcastic comments instead of actually looking into the issue. From the get go you were sarcastic until proven otherwise. I spend time and effort writing stuff so you can improve the library, I don't need your medieval sarcasm, nobody does.

If you want to look at the issue and improve your library fine, if not good luck. I`m not wasting my time anymore.

@Murderlon
Copy link
Member

Sir, let one thing be very clear, you are using a free product, you don't have any right to customer support. This is all based on best effort. You started your issue of with:

Well for starters, I would like to have [...] Then I would like to have [...]

We are not waiters at a restaurant serving at your command. Open source software is driven by the community and kindness. You then resumed to ignore my answers with references to docs and asked the same question again. Now, once again, you didn't read my comment:

The initial HEAD request is also expected when you are resuming an upload or when an previous upload was found

Two things can happen now, you don't use this awesome product that takes away massive complexity you'd have to write yourself, or we start again like this:

  1. Issues are created with as much detail as possible
  2. We try our best to help you and figure out if this is actually an issue
  3. If you have a different unrelated question, we prefer it to be asked in the community forum or in case of a bug create a new issue. This is to have a shared place for knowledge and to make issues talk about the same thing, not multiple things.

Lastly, It wasn't my intent to come off as sarcastic or uncaring, sorry if that happened.

@jonathanlal
Copy link

onBeforeRequest is triggered for each HTTP request made by the TUS protocol (including each chunk of the file being uploaded).. so your api request to create a videoId will fire multiple times (for each chunk of your file).. I am facing this issue right now and not sure how to resolve it..

@Murderlon
Copy link
Member

Could it be that you are setting a chunkSize? If so, we highly recommend to not set it as per the docs. Maybe that helps already.

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

Successfully merging a pull request may close this issue.

3 participants