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

Uploading files with a content size >~ 2.1 GB do not restart properly #5468

Open
kaloramik opened this issue Dec 1, 2024 · 29 comments
Open
Labels
bug Something isn't working pending-triage Issue is pending triage

Comments

@kaloramik
Copy link

kaloramik commented Dec 1, 2024

Describe the bug

I am utilizing the behavior in the Swift TransferUtility documentation here https://docs.amplify.aws/gen1/swift/sdk/storage/transfer-utility/#managing-transfers-when-an-app-restarts

I have tested this with a 1.75 GB file and a 2.3 GB file. It seems to work correctly for the 1.75 GB file, but not for the 2.3 GB file.

In the 2.3 GB case, the contentLength gets changed to something negative. One possible hypothesis could be that 2.3 GB is larger than the size of a 32 bit signed int?

To Reproduce
Base case

  1. Try uploading a file of size < 2.1 GB
  2. use Fast App Termination and put app in background to simulate the app getting terminated by the system
  3. re-open app
  4. after a few minutes, the app correctly associates the upload and finishes

Steps to reproduce the behavior:

  1. Try uploading a file of size > 2.1 GB
  2. use Fast App Termination and put app in background to simulate the app getting terminated by the system
  3. re-open app
  4. after a few minutes, you can see onProgress callbacks, but the progress is always 0 and task.contentLength is negative
  5. Eventually, the upload will fail with the following error
com.amazonaws.AWSS3TransferUtilityErrorDomain - 2: Error Domain=com.amazonaws.AWSS3TransferUtilityErrorDomain Code=2 "(null)" UserInfo={Message=Expected to send [-1962180493], but sent [2332786803] and there are no remaining parts. Failing transfer }

Observed Behavior
see above
Expected Behavior

  • my expected behavior is that the upload would restart more quickly than 3 minutes, or at least have some way to indicate to the app that it is restarting, but that bug is already filed here Long delay in resuming multi-part uploads after restarting app #4580. I suspect if one were to terminate the app again while the restart is happening, this would incur more issues, but that's separate from this issue.

A clear and concise description of what you expected to happen.

Stack Trace
Please provide a stack trace if applicable e.g. a crash is occurring.

I am logging to Datadog here

  private func handleProgress(task: MultiPartUploadTask, progress: Progress) {
    NavigateDD.logger?.info("in handle progress \(task.file) \(task.location) \(task.file) \(task.status) \(task.error) \(task.description) \(task.key) \(task.contentLength) \(task.completedPartsSet.count)")
    let progressParams = ["progress": progress.fractionCompleted]
    let event = self.createEvent(task: task, customParams: progressParams)
    self.sendEvent(withName: PROGRESS_EVENT, body: event)
  }

you can see here where after the app restarts, the contentLength changes
Screenshot 2024-12-01 at 1 09 38 PM

Note that the original content length was 2332786803. the new content length is -1962180493 This is the value of the former as a signed int

>>> def get_signed_value(unsigned_int):
...     signed_value = large_value & 0xFFFFFFFF
...     if signed_value >= 2**31:
...             signed_value -= 2**32
...     return signed_value
...
>>> get_signed_value(2332786803)
-1962180493

Code Snippet
I have generally followed the same instructions in the AutoResume sample app

Unique Configuration
If you are reporting an issue with a unique configuration or where configuration can make a difference in code execution (i.e. Cognito) please provide your configuration. Please make sure to obfuscate sensitive information from the configuration before posting.

Areas of the SDK you are using (AWSMobileClient, Cognito, Pinpoint, IoT, etc)?

  • AWSTransferUtility

Screenshots
If applicable, add screenshots to help explain your problem.

Environment(please complete the following information):

  • SDK Version: 2.37.2
  • Dependency Manager: Cocoapods
  • Swift Version : Latest
  • Xcode Version: Latest

Device Information (please complete the following information):

  • Device: iPhone 14 Pro
  • iOS Version: 18
  • Specific to simulators: probably not

Additional context
Add any other context about the problem here like your specific use case.

Relevant Console Output
In some cases, it might be helpful to add any logs from your console that better help you tell the story of this issue. You can easily enable logging in your app by putting the following code in your app delegate.

AWSDDLog.sharedInstance.logLevel = .debug
AWSDDLog.add(AWSDDTTYLogger.sharedInstance)

Logs

Please logs in the code block below. (See: Logs Guidance)

Log Messages
INSERT LOG MESSAGES HERE
@github-actions github-actions bot added pending-triage Issue is pending triage pending-maintainer-response Issue is pending response from an Amplify team member labels Dec 1, 2024
@kaloramik kaloramik changed the title Uploading files that have content size > size of Int do not restart properly Uploading files with a content size > 2.1 GB do not restart properly Dec 1, 2024
@kaloramik kaloramik changed the title Uploading files with a content size > 2.1 GB do not restart properly Uploading files with a content size >~ 2.1 GB do not restart properly Dec 1, 2024
@tylerjroach
Copy link
Member

Thank you for your report. I will go ahead and label this as a bug while we attempt to replicate the issue.

@tylerjroach tylerjroach added the bug Something isn't working label Dec 2, 2024
@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 2, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 2, 2024

@kaloramik Is using Amplify a possibility for you? Amplify uses the latest feature sets and has lot of improvements compared to AWS SDK.

@kaloramik
Copy link
Author

@harsh62 we can, but we already have our own backend / auth setup and would prefer not onboarding on to the entire Amplify ecosystem. Do you have a link to instructions / guide on how to use Amplify as a drop-in replacement for the AWSTransferUtility?

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 2, 2024
@kaloramik
Copy link
Author

kaloramik commented Dec 3, 2024

@harsh62 In addition, background uploading is extremely important for us. Looking at the documentation, the Amplify Upload Storage does not mention behavior when the app goes into suspension, terminated by system, or terminated by user.

https://docs.amplify.aws/swift/build-a-backend/storage/upload-files/

Whereas the gen 1 system seems to lay it out more specifically

https://docs.amplify.aws/gen1/swift/sdk/storage/transfer-utility/

Do you have any guarantees on reliability, especially when it comes to backgrounding, suspension, and restarts for the Amplify version? I would not want to spend a lot of time migrating just to run into issues that were solved in the Gen 1 version.

@creitz
Copy link
Contributor

creitz commented Dec 3, 2024

This could be (one of) the source(s) of the issue. It seems the value for content_length is not being hydrated properly from the DB here:

[transfer setObject:@([rs intForColumn:@"content_length"]) forKey:@"content_length"];

For example, when uploading a file of 2,493,465,483 bytes, this is the value that's shown when rehydrating:
int_for_column

Obviously, -1801501813 is an impossible content length. Changing intForColumn to longForColumn seems to at least fix the reading of that value.
long_for_column

@harsh62
Copy link
Member

harsh62 commented Dec 3, 2024

@creitz Do you want to open a PR for the fix that you are suggesting? (Looks correct to me)

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 3, 2024
@kaloramik
Copy link
Author

@harsh62 just bumping the previous message on Amplify, would love to get some more info on migrating and the feature set in the new Amplify SDK, especially WRT background uploads

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 3, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 3, 2024

@kaloramik I am working with my team to draft documentation around your question.

backgrounding, suspension, and restarts for the Amplify version

I will get back to you on this soon

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 3, 2024
@kaloramik
Copy link
Author

awesome thank you!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 3, 2024
@creitz
Copy link
Contributor

creitz commented Dec 4, 2024

@creitz Do you want to open a PR for the fix that you are suggesting? (Looks correct to me)

Sure can do!

@creitz
Copy link
Contributor

creitz commented Dec 4, 2024

@harsh62 @kaloramik I've opened #5471

@harsh62
Copy link
Member

harsh62 commented Dec 4, 2024

@creitz Thanks.. I am trying to resolve a Github Actions issue with the repo and I'll get this merged once that is resolved.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 4, 2024
@creitz
Copy link
Contributor

creitz commented Dec 6, 2024

Hi @harsh62, any updates on the above? Thanks!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 6, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 6, 2024

@creitz I am actively working on it. Not able to provide an ECD on it yet, but will keep the ticket updated of what the progress is.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 6, 2024
@creitz
Copy link
Contributor

creitz commented Dec 6, 2024

@harsh62 appreciated!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 6, 2024
@creitz
Copy link
Contributor

creitz commented Dec 6, 2024

Also @harsh62, if the fix was merged today (or soon), when would you expect it to be in a live release? Are releases done on a schedule, or ad-hoc? Judging by AWSS3.podspec, I can't quite tell.

@harsh62
Copy link
Member

harsh62 commented Dec 6, 2024

@creitz
The releases are adhoc. And if the fix is merged today, I'll be able to cut a release Monday morning.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 6, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 9, 2024

@creitz So the pipeline fix has been merged, I updated your PR and ran the integration and unit tests..

@creitz
Copy link
Contributor

creitz commented Dec 9, 2024

@harsh62 Great, thanks!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 9, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 9, 2024

@creitz Release is in progress.. SPM is complete, Cococapods might take a couple hours more.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 9, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 9, 2024

@kaloramik

So my teammate came back and they confirmed that Amplify V2 doesn't have the same level of features that were available in TransferUtility. This is something that Amplify team will prioritize in the coming releases.

For now, I hope @creitz contribution could fix your issue.

@kaloramik
Copy link
Author

I see, Thanks for the update @harsh62. For my own information, is the Transfer Utility / aws ios SDK something you guys are planning on supporting long term? My ideal would be that it is a library on its own that Amplify V2 calls under the hood, and so it would be maintained / upgraded as a part of Amplify vs ditching it for an alternative.

Also, is there a way for us to use the Amplify Upload Storage capabilities without having to onboard onto the full Amplify Stack? (AKA using it as a library as we do now)

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 9, 2024
@creitz
Copy link
Contributor

creitz commented Dec 9, 2024

@creitz Release is in progress.. SPM is complete, Cococapods might take a couple hours more.

Thanks! For my information, would we expect this file to be updated for the release then?

@harsh62
Copy link
Member

harsh62 commented Dec 9, 2024

Thanks! For my information, would we expect this file to be updated for the release then?

Yes. You'll see new release commits get pushed onto the repo.. The Cocoapods release is still in progress.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 9, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 9, 2024

@kaloramik Long term plan is to use Swift SDK in favor of this repo. The team will share plans on the future of this repo soon. As for why Transfer Utility features are not available is something that the team is discussing internally (probably a miss when Amplify V2 was created).

For the 2nd question, you can use Amplify Storage or any other plugin without setting up Amplify Stack as follows:

        func addAmplifyWithStorage() throws {
            try Amplify.add(plugin: AWSS3StoragePlugin())
            let configuration = AmplifyConfiguration(
                storage: StorageCategoryConfiguration(
                    plugins: [
                        "awsS3StoragePlugin": [
                            "bucket": "[BUCKET NAME]",
                            "region": "[REGION]"
                        ]
                    ]
                )
            )
            try Amplify.configure(configuration)
        }

You wanna probably add Auth too as Storage fetches credentials from there (user pool and/or identity pool), its a similar process:

            let configuration = AmplifyConfiguration(
                auth: AuthCategoryConfiguration(
                    plugins: [
                        "awsCognitoAuthPlugin": [
                            "CognitoUserPool": [
                                "Default": [
                                    "PoolId": "[POOL_ID]",
                                    "AppClientId": "[APP_CLIENT_ID]",
                                    "Region": "[REGION]"
                                ]
                            ],
                            "CredentialsProvider": [
                                "CognitoIdentity": [
                                    "Default": [
                                        "PoolId": "[IDENTITY_POOL_ID]",
                                        "Region": "[REGION]"
                                    ]
                                ]
                            ]
                        ]
                    ]
                ),
                storage: StorageCategoryConfiguration(
                    plugins: [
                        "awsS3StoragePlugin": [
                            "bucket": "[BUCKET NAME]",
                            "region": "[REGION]"
                        ]
                    ]
                )
            )

@creitz
Copy link
Contributor

creitz commented Dec 9, 2024

Oh interesting, what is the status of the Swift SDK you mentioned? Would you recommend migration to that now (is there parity with the aws-sdk-ios library?), or is it similar to the amplify repo in that parity is not quite there?

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 9, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 10, 2024

@creitz

The AWS Swift SDK is now generally available and suitable for production use. In fact, Amplify Swift relies on the Swift SDK as a dependency under the hood.

It’s important to note that the AWS SDK for iOS and the AWS SDK for Swift are fundamentally different and not interchangeable. Depending on the features you need, you could either use Amplify (leveraging the Swift SDK for unsupported features) or directly adopt the AWS SDK for Swift and implement your own logic where necessary.

The AWS SDK for Swift provides a direct interface to AWS APIs, plus additional platform-specific functionality. Amplify Swift, on the other hand, is an abstraction layer that builds its own APIs on top of the Swift SDK—sometimes involving multiple AWS APIs to deliver a single feature.

Because these approaches differ substantially, there isn’t a straightforward, one-size-fits-all migration guide. Your migration strategy will depend on your specific requirements and goals. I recommend exploring your options and planning your approach as soon as possible.

@github-actions github-actions bot removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 10, 2024
@harsh62
Copy link
Member

harsh62 commented Dec 10, 2024

@creitz 2.38.0 is available via Cocoapods

@creitz
Copy link
Contributor

creitz commented Dec 10, 2024

@harsh62 thanks for the details and for letting me know!

@github-actions github-actions bot added the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 10, 2024
@harsh62 harsh62 removed the pending-maintainer-response Issue is pending response from an Amplify team member label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pending-triage Issue is pending triage
Projects
None yet
Development

No branches or pull requests

4 participants