muxinc / upchunk

Uploads Chunks! Takes big files, splits them up, then uploads each one with care (and PUT requests).
MIT License
340 stars 46 forks source link

allow sendChunk override #86

Closed DmitryKashinskyAtSpark closed 2 years ago

DmitryKashinskyAtSpark commented 2 years ago

While integrating UpChunk it might be necessary to adjust headers or do something else right before every chunk is sent over the network. For example, when other than mux backend is used, it is necessary to make sure that auth tokens are valid on every network call. The most flexible option could be additional UpChunkOption to provide such onBeforeSend callback and execute it later. However, inheritance can also be used and this one looks like the minimal change to the existing code.

Here is the usage example which will be less hacky and much safer after PR is approved and merged:

class UpChunkExtended extends UpChunk {
    /**
     * DK: that's a real hack here.
     * We override private method in order to make sure that token is valid before sending any chunk.
     * I'll create follow-up pull request to original repository to make it properly
     *
     * @returns result of the call to the base class sendChunk method
     */
    private async sendChunk() {
        const token = await authApi.getOrRefreshAccessToken();

        this.headers['Authorization'] = 'Bearer ' + token;
        this.headers['x-functions-key'] = filesFunctionKey;

        // @ts-ignore
        return super.sendChunk();
    }
}
dylanjha commented 2 years ago

@DmitryKashinskyAtSpark does the headers option not sufficiently solve this for you? link

Headers specified there would be included on every PUT request.

DmitryKashinskyAtSpark commented 2 years ago

@dylanjha the thing here is that chunks uploading is completely internal and can take quite long for some files. Imagine user uploads some video file from mobile device on slow internet connection, while auth token is short living. If upload takes 5 minutes and auth token lives for 1 minute user will never be able to complete this upload. In terms of code, we can manage headers propery as it is publicly accessible and explicitly set it when token is updated, but this update does not happen automatically. Since we never know if we are going to make request or not, ensuring token validity right before sending chunk feels to be the best place to do so.

Hope my explanation and reasoning is clear :)

dylanjha commented 2 years ago

@DmitryKashinskyAtSpark makes perfect sense 👍🏼. You need to refresh the access token for each request so the static headers are not sufficient.

DmitryKashinskyAtSpark commented 2 years ago

@cjpillsbury @dylanjha Assuming everything is good to go, could someone please merge this PR 🚀 ? It appears I have no rights to do so.

cjpillsbury commented 2 years ago

Done. Will try to get a release out asap (we may be making more of these methods protected for greater versatility of usage, including testing stubs/mocks).