aws-amplify / aws-sdk-ios

AWS SDK for iOS. For more information, see our web site:
https://aws-amplify.github.io/docs
Other
1.68k stars 884 forks source link

Cannot upload images to S3 anymore #4534

Closed dcristolovean closed 1 year ago

dcristolovean commented 1 year ago

Describe the bug The issue appeared 1-2 months ago, can't pinpoint it exactly.

I'm using AWSS3TransferUtility and uploadImage or uploadUsingMultiPart (both have the same behavior) and I run into these scenarios:

  1. The upload works (rarely, but it does. Also, with the same settings, same everything, it works from the Android SDK, S3 config is good)
  2. The upload method never returns anything, worst case scenario, I have a spinner that never closes.
  3. The upload method returns an "unknown error" without much explanation

When the error occurs, the AWSS3TransferUtility seems to dump some errors in the logs:

Line 5: BackgroundSession attempted to create a NSURLSessionUploadTask in a session that has been invalidated Line 12: Exception in upload task Task created in a session that has been invalidated Line 66: Error: Error Domain=com.amazonaws.AWSS3TransferUtilityErrorDomain Code=0 "(null)" UserInfo={Message=Exception from upload task., Reason=Task created in a session that has been invalidated}

It's also not a case of authentication, everything else works, other AWS SDKs I'm using are OK.

Expected Behavior The image should upload and the handler should be called, with OK or with an error.

Stack Trace Line 5: BackgroundSession attempted to create a NSURLSessionUploadTask in a session that has been invalidated Line 12: Exception in upload task Task created in a session that has been invalidated Line 66: Error: Error Domain=com.amazonaws.AWSS3TransferUtilityErrorDomain Code=0 "(null)" UserInfo={Message=Exception from upload task., Reason=Task created in a session that has been invalidated}

Code Snippet func uploadImage(data: Data, bucket: String, key: String, encodeId: String, success: @escaping (Bool) -> (), failure: @escaping (Error?) -> ()) { AWSS3TransferUtility.bFan().uploadUsingMultiPart(data: data, bucket: bucket, key: key, contentType: "image/png", expression: nil) { result, error in if let err = error { failure(err) } else { success(true) } }

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)? AWSS3

Environment(please complete the following information):

Device Information (please complete the following information):

jcjimenez commented 1 year ago

Hi @dcristolovean, as you noted, it looks like the AWSS3TransferUtility's underlying NSURLSession is being invalidated before an attempt to upload is done. So, pardon me if these questions are a bit remedial - just want to make sure I understand:

  1. In your code snippet above, does bFan represent an extension added outside the aws-sdk-ios library? If so, which of AWSS3TransferUtility's initialization methods does it use? I ask because if, for example, S3TransferUtilityForKey or registerS3TransferUtilityWithConfiguration:* variants are being used along with removeS3TransferUtilityForKey:, it's possible that is how the underlying NSURLSession is getting invalidated.
  2. Is any code, by chance, attempting to get ahold of the AWSS3TransferUtility.session property, e.g. [utility valueForKey:@"session"]? If so, it's possible somewhere in a related codepath, the session's invalidate method's are getting called.
  3. Can you try subscribing to the AWSS3TransferUtilityURLSessionDidBecomeInvalidNotification notification and see when that is getting posted? That may give us an idea for the origin of the invalidation. If you never see that notification, that could also mean that something is overriding the NSURLSession's delegate.
dcristolovean commented 1 year ago
  1. Yes, it's just a simple extension I have to easily access a static client

`extension AWSS3TransferUtility {

@objc static func bFan() -> AWSS3TransferUtility {
    return AWSS3TransferUtility.s3TransferUtility(forKey: AWSS3TransferUtility.key)!
}

@objc static func removeBFanClient() {
    return AWSS3TransferUtility.remove(forKey: AWSS3TransferUtility.key)
}

@objc static func registerBFanClient(_ configuration: AWSServiceConfiguration, transferUtilityConfiguration: AWSS3TransferUtilityConfiguration?) {
    AWSS3TransferUtility.key = String.random(length: 15)
    if let tr = transferUtilityConfiguration {
        AWSS3TransferUtility.register(with: configuration, transferUtilityConfiguration: tr, forKey: AWSS3TransferUtility.key)
    }
    else {
        AWSS3TransferUtility.register(with: configuration, forKey: AWSS3TransferUtility.key)
    }
}`
  1. No, there is just one usage of the AWSTransferUtility in the whole app (except creating those clients from above) and that's the problematic uploadData or uploadMultipart

  2. I tried it like this

NotificationCenter.default.addObserver(self, selector: #selector(self.sessionFailed), name: NSNotification.Name.AWSS3TransferUtilityURLSessionDidBecomeInvalid, object: nil)

but nothing calls the sessionFailed method. Should I do it differently somehow ? Not sure what can override the NSURLSession delegate, since it's an internal one that AWSTransferUtility creates.

To summarize:

2023-02-03 20:17:49.855434+0200 bfanteam[43876:2553731] BackgroundSession <B262610C-E2C8-488D-8CC6-CD2C4BB5FC48> an error occurred on the xpc connection to setup the background session: Error Domain=NSCocoaErrorDomain Code=4097 "connection to service named com.apple.nsurlsessiond" UserInfo={NSDebugDescription=connection to service named com.apple.nsurlsessiond} 2023-02-03 20:17:49.856438+0200 bfanteam[43876:2553731] BackgroundSession <B262610C-E2C8-488D-8CC6-CD2C4BB5FC48> failed to create a background NSURLSessionUploadTask, as remote session is unavailable 2023-02-03 20:17:49.860161+0200 bfanteam[43876:2553731] Task <784A1CFE-0A66-41BC-92D3-03EDB857F0F1>.<1> finished with error [-1] Error Domain=NSURLErrorDomain Code=-1 "unknown error" UserInfo={NSErrorFailingURLStringKey=https://sportarchive-qa-eu-orgs.s3-accelerate.amazonaws.com/users_profile_pictures/bfanteam/eu-west-1_580d5d60-a913-488f-8595-cb7ebb35da99/default/1675448265299445a4c3194233663.png?uploadId=RlwqNwZ_Uya7IKn.tw4iDHb73zPVPfAXTGp0zScGHqicVxaU7uqiI7mk0JAJoqJQxzNvKPL3zJBLArDK2n_EGGoGcN2zwuRQ7K62Q86GVPD_yoGklaiE.myXAV9etrMYmUSpXlYwf.2hSXhCHLpXd1IcAsqgXVn9ii5nD2hDZZ8-&partNumber=1&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAZMEXNFDZCATU6GVF%2F20230203%2Feu-west-1%2Fs3%2Faws4_request&X-Amz-Date=20230203T181749Z&X-Amz-Expires=2999&X-Amz-SignedHeaders=content-type%3Bhost&X-Amz-Security-Token=**LONG TOKEN HERE**-Amz-Signature=bfebd488eae0f7ce62a708177f30be3ff1513928340941cc07bf30e35621e4ec, NSErrorFailingURLKey=https://sportarchive-qa-eu-orgs.s3-accelerate.amazonaws.com/users_profile_pictures/bfanteam/eu-west-1_580d5d60-a913-488f-8595-cb7ebb35da99/default/1675448265299445a4c3194233663.png?uploadId=RlwqNwZ_Uya7IKn.tw4iDHb73zPVPfAXTGp0zScGHqicVxaU7uqiI7mk0JAJoqJQxzNvKPL3zJBLArDK2n_EGGoGcN2zwuRQ7K62Q86GVPD_yoGklaiE.myXAV9etrMYmUSpXlYwf.2hSXhCHLpXd1IcAsqgXVn9ii5nD2hDZZ8-&partNumber=1&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=ASIAZMEXNFDZCATU6GVF%2F20230203%2Feu-west-1%2Fs3%2Faws4_request&X-Amz-Date=20230203T181749Z&X-Amz-Expires=2999&X-Amz-SignedHeaders=content-type%3Bhost&X-Amz-Security-Token=<**LONG TOKEN HERE**-Amz-Signature=bfebd488eae0f7ce62a708177f30be3ff1513928340941cc07bf30e35621e4ec, _NSURLErrorRelatedURLSessionTaskErrorKey=( "BackgroundUploadTask <784A1CFE-0A66-41BC-92D3-03EDB857F0F1>.<1>" ), _NSURLErrorFailingURLSessionTaskErrorKey=BackgroundUploadTask <784A1CFE-0A66-41BC-92D3-03EDB857F0F1>.<1>, NSLocalizedDescription=unknown error}

This upload works with our Android app and it also worked (I'd say for years now) on iOS until somewhere last year ? (I'd say around when iOS 16 started to be adopted). I'm also using now the latest version, 2.30.1 I think.

And one last question: since we're using the 'old' approach, with AWSMobileClient, AWSTransferUtility, etc, can I move to Amplify.Storage, at least for the S3 part ? I'm supporting >= iOS 13, so I can use Amplify if needed and if it can be set up like that.

jcjimenez commented 1 year ago

Thanks, @dcristolovean - let me look a little closer into this. However, one more question:

Is AWSS3TransferUtility.key defined by the library? (I don't see it in main branch) The reason I ask is because it looks like a static value, and URLSessions must have a unique identifier (which AWSS3TransferUtility creates using the key you pass in) each time you create one. If the same identifier was used in a previous execution of your application (or app extension), it may be possible you're referencing a stale URL session despite instantiating a brand-new AWSS3TransferUtility. I suggest you use a unique identifier per lifespan of user app and only release it when you are sure you're no longer going to need it. Either that, or try defaultS3TransferUtility.

I tried it like this NotificationCenter.default.addObserver(self, selector: #selector(self.sessionFailed), name: NSNotification.Name.AWSS3TransferUtilityURLSessionDidBecomeInvalid, object: nil) but nothing calls the sessionFailed method. Should I do it differently somehow ?

Your code looks correct to me. It makes me think that either the session got invalidated before your app ran, the AWSS3TransferUtility isn't getting its delegate method called (because the delegate got overridden somehow?), or the invalidation isn't happening (but unlikely).

And one last question: since we're using the 'old' approach, with AWSMobileClient, AWSTransferUtility, etc, can I move to Amplify.Storage, at least for the S3 part ? I'm supporting >= iOS 13, so I can use Amplify if needed and if it can be set up like that.

From what I see here, I can't see a reason not give this a try! (but first, I suggest trying thing I mentioned above relating to the session identifier).

dcristolovean commented 1 year ago

The key is created in the code I provided

AWSS3TransferUtility.key = String.random(length: 15)

random(length: ..) is just my own method that generates a random string for a given length. So the key is completely random each time.

It's also a property in the extension, since one can't have variables defined in the extension:

`struct AWSS3TransferUtilityAssociatedKeys { static var key: UInt8 = 0 }

extension AWSS3TransferUtility { @objc static var key: String { get { guard let value = objc_getAssociatedObject(self, &AWSS3TransferUtilityAssociatedKeys.key) as? String else { return "" } return value } set(newValue) { objc_setAssociatedObject(self, &AWSS3TransferUtilityAssociatedKeys.key, newValue, objc_AssociationPolicy.OBJC_ASSOCIATION_RETAIN_NONATOMIC) } }`

It's used so I can also access it from the bfan() and removeBfanClient() methods. It's surely unique and created and used for register just once.

Not sure what else to try, except moving to Amplify.

jcjimenez commented 1 year ago

Got it! Yeah, so, your utility and session should be unique for each launch.

One more thing, you are setting AWSS3TransferUtility.key = String.random(length: 15) once and only once and calling AWSS3TransferUtility.bFan() + AWSS3TransferUtility.removeBFanClient() more than once?

dcristolovean commented 1 year ago

it's set once, in the register and then only used to retrieve the client and call the upload method

@objc static func bFan() -> AWSS3TransferUtility { return AWSS3TransferUtility.s3TransferUtility(forKey: AWSS3TransferUtility.key)! }

It's also never removed during normal app functionalities. Only at logout/login, which is not the case here.

jcjimenez commented 1 year ago

Got it, thanks, @dcristolovean - looking into this further.

dcristolovean commented 1 year ago

Thank you. In the meantime, I'm looking into the possibility of just using Amplify.Storage for S3 while keeping everything else the same, at least for now.

jcjimenez commented 1 year ago

Sounds great, please let us know how that goes.

koxon commented 1 year ago

Looks like the error starts here with xpc connection error:

`2023-02-03 20:17:49.855434+0200 bfanteam[43876:2553731] BackgroundSession an error occurred on the xpc connection to setup the background session: Error Domain=NSCocoaErrorDomain Code=4097 "connection to service named com.apple.nsurlsessiond" UserInfo={NSDebugDescription=connection to service named com.apple.nsurlsessiond}

Followed with this one from the SDK: 2023-02-03 20:17:49.856438+0200 bfanteam[43876:2553731] BackgroundSession failed to create a background NSURLSessionUploadTask, as remote session is unavailable`

I'm working with @dcristolovean on this and we still have the issue randomly. We can't find the source of the issue.

dcristolovean commented 1 year ago

I also used Amplify.Storage to upload the files (a conbination between Amplify v1 and AwsMobileClient) and I got the exact same behavior as above.

Amplify v2 is out of the question at the moment, the migration is extremely difficult.

So the problem persist, same image might work one day then it doesn't the next day and I see that internal AWS error mentioned above.

jcjimenez commented 1 year ago

Hi, @koxon , @dcristolovean,

Have you, by chance, tried this workaround?

  1. Create an instance of a AWSS3PreSignedURLBuilder:
    [AWSS3PreSignedURLBuilder registerS3PreSignedURLBuilderWithConfiguration:configuration forKey:key];
    AWSS3PreSignedURLBuilder *builder = [AWSS3PreSignedURLBuilder S3PreSignedURLBuilderForKey:key];
  2. Pass it a AWSS3GetPreSignedURLRequest:
    AWSS3GetPreSignedURLRequest *request = [AWSS3GetPreSignedURLRequest new];
    request.bucket = ...;
    request.key = ...;
    request.HTTPMethod = AWSHTTPMethodPUT;
    request.expires = [NSDate dateWithTimeIntervalSinceNow:3600];
  3. Get an NSURL from the builder and pass it to your NSURLSession:
    [[[builder getPreSignedURL:getPreSignedURLRequest] continueWithBlock:^id(AWSTask *task) {
        NSURL *presignedURL = task.result;
        // Ask your NSURLSesion to upload to that URL
    }] waitUntilFinished];

I suspect something about the management of the background NSURLSession with the given ids may not be working, but may work for the AWSS3PreSignedURLBuilder.

Keep in mind, this is for a single PutObject operation which has a 5GB limit but should be more than enough for images:

https://docs.aws.amazon.com/AmazonS3/latest/userguide/upload-objects.html

dcristolovean commented 1 year ago

This finally worked ! So if anyone runs into this:

  1. This doesn't work: (I'm not doing anything else besides this, not touching any URLSession in AWS and so on)

AWSS3TransferUtility.bFan().uploadUsingMultiPart(data: data, bucket: bucket, key: key, contentType: "image/png", expression: nil) { result, error in .... }

  1. This works:

    `let signedRequest = AWSS3GetPreSignedURLRequest() signedRequest.bucket = bucket signedRequest.key = key signedRequest.httpMethod = .PUT signedRequest.expires = Date(timeIntervalSinceNow: 3600)

    builder.getPreSignedURL(signedRequest).continueWith { task in
        let url = URL(string: task.result?.absoluteString ?? "")
    
        var request = URLRequest(url: url!)
        request.httpMethod = "PUT"
        request.setValue("image/png", forHTTPHeaderField: "Content-Type")
    
        let task = URLSession.shared.uploadTask(with: request, from: data) { data, response, error in
    
            if let e = error {
                failure(e)
            }
            else {
                success(true)
            }
    
        }
    
        task.resume()
    
        return nil
    }`

No idea why this happens, if it's a bug in the SDK or something else.

jcjimenez commented 1 year ago

I'm glad this worked, @dcristolovean!

BrettyWhite commented 6 months ago

This is still an issue even as of version 2.36.2 closing it because someone found a roll-your-own workaround still doesn't fix the underlying cause in the library