Skip to content

Add abort signal to upload#106

Merged
dajimenezriv-internxt merged 5 commits intomasterfrom
add-abort-signal-to-upload
Mar 6, 2026
Merged

Add abort signal to upload#106
dajimenezriv-internxt merged 5 commits intomasterfrom
add-abort-signal-to-upload

Conversation

@dajimenezriv-internxt
Copy link
Contributor

@dajimenezriv-internxt dajimenezriv-internxt commented Mar 5, 2026

What

This PR goes together with this one: internxt/sdk#373. Currently we have some users that are saying that after aborting upload some files in a corrupted state. Looking at the code I think that cannot happen because of the abort, but it's true that some of the requests related to the upload are not being aborted.

Since I was touching this code I decided to make a proposal by changing a bit how the upload function works, by instead of returning an ActionState that can stop(), we accept an abortSignal like when doing requests and what the upload function does is to return a fileId or throw an error. It will change how the upload function is consumed, so I want to be sure that makes sense to make the proposal before going more deep and testing everything.

I think that this new way will give more control to the client using the upload by letting the client decide how to manage the abortSignal and by just returning the fileId value instead of using callbacks.

What do you think?

[Update] Join upload and uploadMultipart functions and expose just one upload function.

@dajimenezriv-internxt dajimenezriv-internxt self-assigned this Mar 5, 2026
@dajimenezriv-internxt dajimenezriv-internxt requested review from jzunigax2, larryrider and sg-gs and removed request for jzunigax2 and larryrider March 5, 2026 12:14
Copy link
Member

@sg-gs sg-gs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I will bump the version to 3.0.0 as this would be a breaking change for all the platforms as now the upload is a promise rather than a callback and the abort controller should be injected by the client itself

@sg-gs sg-gs added the enhancement New feature or request label Mar 5, 2026
@dajimenezriv-internxt
Copy link
Contributor Author

@sg-gs Okey, for now it was just the proposal. First it will be to merge the one from the sdk, then update the sdk version here and do more tests in windows to check that everything is right. So first is this PR: internxt/sdk#373

@dajimenezriv-internxt dajimenezriv-internxt merged commit 34701a7 into master Mar 6, 2026
2 checks passed
@dajimenezriv-internxt dajimenezriv-internxt deleted the add-abort-signal-to-upload branch March 6, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants