Merge pull request #17759 from wordpress-mobile/fix/media-error-crash

Fix Core Data crash related to media errors

RELEASE-NOTES.txt

@@ -16,6 +16,7 @@
 * [*] Block editor: Fix cut-off setting labels by properly wrapping the text [https://github.com/wordpress-mobile/gutenberg-mobile/pull/4475]
 * [*] Block editor: Highlight text: fix applying formatting for non-selected text [https://github.com/wordpress-mobile/gutenberg-mobile/pull/4471]
 * [**] Block editor: Fix Android handling of Hebrew and Indonesian translations [https://github.com/wordpress-mobile/gutenberg-mobile/pull/4397]
+* [***] Self-hosted sites: Fixed a crash when saving media and no Internet connection was available. [#17759]
 
 19.0
 -----

WordPress/Classes/Models/Media.m

@@ -261,4 +261,18 @@ - (BOOL)hasRemote {
     return self.mediaID.intValue != 0;
 }
 
+- (void)setError:(NSError *)error
+{
+    if (error != nil) {
+        // Cherry pick keys that support secure coding. NSErrors thrown from the OS can
+        // contain types that don't adopt NSSecureCoding, leading to a Core Data exception and crash.
+        NSDictionary *userInfo = @{NSLocalizedDescriptionKey: error.localizedDescription};
+        error = [NSError errorWithDomain:error.domain code:error.code userInfo:userInfo];
+    }
+
+    [self willChangeValueForKey:@"error"];
+    [self setPrimitiveValue:error forKey:@"error"];
+    [self didChangeValueForKey:@"error"];
+}
+
 @end

WordPress/Classes/Services/MediaService.m

@@ -425,7 +425,7 @@ - (NSError *)customMediaUploadError:(NSError *)error remote:(id <MediaServiceRem
                 case NSURLErrorNetworkConnectionLost:
                 case NSURLErrorNotConnectedToInternet:
                     // Clear lack of device internet connection, notify the user
-                    customErrorMessage = NSLocalizedString(@"The internet connection appears to be offline.", @"Error message shown when a media upload fails because the user isn't connected to the internet.");
+                    customErrorMessage = NSLocalizedString(@"The Internet connection appears to be offline.", @"Error message shown when a media upload fails because the user isn't connected to the Internet.");
                     break;
                 default:
                     // Default NSURL error messaging, probably server-side, encourage user to try again
@@ -434,11 +434,13 @@ - (NSError *)customMediaUploadError:(NSError *)error remote:(id <MediaServiceRem
             }
         }
     }
+
     if (customErrorMessage) {
-        NSMutableDictionary *userInfo = [[NSMutableDictionary alloc] init];
+        NSMutableDictionary *userInfo = [error.userInfo mutableCopy];
         userInfo[NSLocalizedDescriptionKey] = customErrorMessage;
-        error = [[NSError alloc] initWithDomain:error.domain code:error.code userInfo:userInfo];
+        error = [NSError errorWithDomain:error.domain code:error.code userInfo:userInfo];
     }
+
     return error;
 }
 

WordPress/WordPressTest/CoreDataMigrationTests.m

@@ -1322,7 +1322,10 @@ - (void)testMigrationFrom103To104CustomTransformers
     // Media has an Error transformer
     Media *media1 = (Media *)[NSEntityDescription insertNewObjectForEntityForName:[Media entityName] inManagedObjectContext:context];
     media1.blog = blog1;
-    NSError *error1 = [NSError errorWithDomain:NSURLErrorDomain code:100 userInfo:@{ @"reason": @"test" }];
+    // The UserInfo dictionary of an NSError can contain types that can't be securely coded, which will throw a Core Data exception on save.
+    // We attach an NSUnderlyingError with the expectation that it won't be included when the error is encoded and persisted.
+    NSError *underlyingError = [NSError errorWithDomain:NSURLErrorDomain code:500 userInfo:nil];
+    NSError *error1 = [NSError errorWithDomain:NSURLErrorDomain code:100 userInfo:@{ NSLocalizedDescriptionKey: @"test", NSUnderlyingErrorKey: underlyingError }];
     media1.error = error1;
 
     [context save:&error];
@@ -1356,7 +1359,9 @@ - (void)testMigrationFrom103To104CustomTransformers
 
     // Check that our properties persisted
     Media *fetchedMedia1 = [context fetch:@"Media" withPredicate:nil arguments:nil].firstObject;
-    XCTAssert([fetchedMedia1.error isEqual:error1]);
+    // The expected error is stripped of any keys not included in the Media.error setter
+    NSError *expectedError = [NSError errorWithDomain:NSURLErrorDomain code:100 userInfo:@{ NSLocalizedDescriptionKey: @"test" }];
+    XCTAssert([fetchedMedia1.error isEqual:expectedError]);
 
     Blog *fetchedBlog1 = [context fetch:@"Blog" withPredicate:@"blogID = %i" arguments:@[@123]].firstObject;
     XCTAssertNotNil(fetchedBlog1);