Consistency errors #1237

flovilmart · 2018-01-19T23:25:32Z

This PR addresses those crashes that started happening since bumping to Bolts 1.9.0.

Starting Bolts 1.9.0, the NSExceptions are not caught by Bolts anymore, and need to be handled differently.

The best way to handle errors is to go the old style objective-c error pointer passing strategy.

This impact a large portion of the codebase as we need to forward errors from the coder / decoder.

On the public side, this should not change anything, as all failable calls will be caught by an errored BFTask.

flovilmart · 2018-01-20T15:13:39Z

@montymxb as of now, it looks like all the PFConsistencyAsset calls are 'safe' and reporting proper errors to the users when 'developing'.

Errors that would occur in flight (when wrapped into an async task have been replaced by error pointers).

flovilmart · 2018-01-20T16:11:11Z

Additionally @marcgovi, could you have a quick look?

flovilmart · 2018-01-20T21:55:24Z

@montymxb as it stands now, I made a pre release 1.17.0-alpha.1 in order to be able to test it out in the wild, I also updated liveQuery as this project relies on the decoders which have changed their interface in order to support encoding failures. I tested on a project and it all look great.

mt81

Minor nits then LGTM

mt81 · 2018-01-22T18:31:16Z

Parse/Parse/Internal/Analytics/Controller/PFAnalyticsController.m

-        NSDictionary *encodedDimensions = [[PFNoObjectEncoder objectEncoder] encodeObject:dimensions];
+        NSError *error;
+        NSDictionary *encodedDimensions = [[PFNoObjectEncoder objectEncoder] encodeObject:dimensions error:&error];
+        if (encodedDimensions == nil && error != nil) {


in objective c we don't have to check for nill != nil, but maybe it is more elegant this way ...

mt81 · 2018-01-22T18:36:41Z

Parse/Parse/Internal/Commands/PFRESTCommand.m

@@ -105,12 +108,13 @@ + (instancetype)commandFromDictionaryRepresentation:(NSDictionary *)dictionary {
    PFRESTCommand *command = [self commandWithHTTPPath:dictionary[PFRESTCommandHTTPPathEncodingKey]
                                            httpMethod:dictionary[PFRESTCommandHTTPMethodEncodingKey]
                                            parameters:dictionary[PFRESTCommandParametersEncodingKey]
-                                          sessionToken:dictionary[PFRESTCommandSessionTokenEncodingKey]];
+                                          sessionToken:dictionary[PFRESTCommandSessionTokenEncodingKey]
+                                                 error:nil];


do we have to add the error handling here?

we should be good if I understand the codepaths correctly

mt81 · 2018-01-24T17:15:38Z

Parse/Parse/Internal/Commands/PFRESTCommand.m

-        self.parameters = [[PFPointerOrLocalIdObjectEncoder objectEncoder] encodeObject:data];
+    BOOL modified = NO;
+    BOOL success = [[self class] forEachLocalIdIn:data doBlock:block modified:&modified error:error];
+    if (!success) {


What do you think of something like this ?

if ([[self class] forEachLocalIdIn:data doBlock:block modified:&modified error:error]) { self.parameters = [[PFPointerOrLocalIdObjectEncoder objectEncoder] encodeObject:data error:error]; if (self.parameters && !(error && *error)) { return YES; } } return NO;

makes sense.

mt81 · 2018-01-24T17:37:52Z

Parse/Parse/Internal/Commands/PFRESTQueryCommand.m

@@ -131,6 +143,8 @@ + (NSDictionary *)findCommandParametersWithOrder:(NSString *)order
        parameters[key] = obj;
    }];

+    __block BOOL encodingHasErrored = NO;


maybe we can use encodingHasErroror encodingFailed?

mt81 · 2018-01-24T17:39:48Z

Parse/Parse/Internal/Commands/PFRESTQueryCommand.m

    if (![object isKindOfClass:[NSDictionary class]]) {
        return object;
    }

    NSMutableDictionary *parameters = [NSMutableDictionary dictionaryWithCapacity:[object count]];
+    __block BOOL hadFailed = NO;


maybe we can use fasFailed or didFail? Up to you...

mt81 · 2018-01-24T17:45:03Z

Parse/Parse/Internal/FieldOperation/PFFieldOperation.m

-    NSMutableArray *encodedObjects = [objectEncoder encodeObject:self.objects];
+- (id)encodeWithObjectEncoder:(PFEncoder *)objectEncoder error:(NSError **)error {
+    NSMutableArray *encodedObjects = [objectEncoder encodeObject:self.objects error: error];
+    if (!encodedObjects && error && *error) {


can we usePFPreconditionBailOnError ?

mt81 · 2018-01-24T17:45:47Z

Parse/Parse/Internal/FieldOperation/PFFieldOperation.m

-    NSMutableArray *encodedObjects = [objectEncoder encodeObject:self.objects];
+- (id)encodeWithObjectEncoder:(PFEncoder *)objectEncoder error:(NSError **)error {
+    NSMutableArray *encodedObjects = [objectEncoder encodeObject:self.objects error:error];
+    if (!encodedObjects && error && *error) {


can we usePFPreconditionBailOnError ?

mt81 · 2018-01-24T17:45:57Z

Parse/Parse/Internal/FieldOperation/PFFieldOperation.m

-    NSMutableArray *encodedObjects = [objectEncoder encodeObject:self.objects];
+- (id)encodeWithObjectEncoder:(PFEncoder *)objectEncoder error:(NSError **)error {
+    NSMutableArray *encodedObjects = [objectEncoder encodeObject:self.objects error:error];
+    if (!encodedObjects && error && *error) {


can we usePFPreconditionBailOnError ?

mt81 · 2018-01-24T17:53:08Z

Parse/Parse/Internal/PFAssert.h

@@ -66,6 +114,7 @@ do {\
                format:description, ##__VA_ARGS__]; \
 } while(0)

+


empty line added... should we remove it?

mt81 · 2018-01-24T18:04:05Z

Parse/Parse/PFEncoder.m

        }
        return newArray;

    } else if ([object isKindOfClass:[NSDictionary class]]) {
        NSMutableDictionary *dict = [NSMutableDictionary dictionaryWithCapacity:[object count]];
+        __block BOOL hasErrored = NO;


hasErrors ?

flovilmart · 2018-01-24T18:28:34Z

thanks @mt81 i'll update with the nits!

- Make Tried to get invalid local id a soft error - Make Tried to save an object with a new, unsaved child. a soft error - Make Tried to save an object with a pointer to a new, unsaved object. a soft error - Make Attempted to find non-existent uuid a soft error

…entUser`

…error

@nlutsenko

- As per discussion over there, @nlutsenko and @richardjrossiii were A1 on the resolution Thanks @czgarrett for the fix Closes #931

- Prevent deadlock by copying estimatedData before traversal

fixes #1083

…issues

… return failed tasks if needed

* 1. Fix typo PFPreconditionFailOnError and PFPreconditionFailAndSetError. 2. If there is a new object in OfflineStore, should not send the exception when the saved object update its OfflineStore cache * Change the condition about newly saved object * fix typo * Revert to PFPreconditionBailOnError and PFPreconditionBailAndSetError

flovilmart force-pushed the consistency-errors branch from 827c95b to 963b2d0 Compare January 19, 2018 23:35

flovilmart requested a review from montymxb January 20, 2018 00:52

flovilmart added type:bug Impaired feature or lacking behavior that is likely assumed unbreak now labels Jan 20, 2018

montymxb mentioned this pull request Jan 20, 2018

Set bolts back to 1.8.4 #1236

Closed

flovilmart force-pushed the consistency-errors branch from cca3587 to c59c775 Compare January 20, 2018 19:11

flovilmart added this to the 1.17.0 milestone Jan 21, 2018

flovilmart closed this Jan 24, 2018

flovilmart reopened this Jan 24, 2018

mt81 approved these changes Jan 24, 2018

View reviewed changes

flovilmart force-pushed the consistency-errors branch 2 times, most recently from 6828a72 to 25a5338 Compare January 24, 2018 18:47

flovilmart closed this Jan 26, 2018

flovilmart reopened this Jan 26, 2018

flovilmart closed this Jan 29, 2018

flovilmart reopened this Jan 29, 2018

flovilmart added 10 commits January 30, 2018 09:40

wip

73b6651

wip

30ffc60

Soft errors for User consistency checks `_checkSaveParametersWithCurr…

4526f07

…entUser`

Bumps to latest Swift

816a6e2

Proper failable methods for the LocalIdStore

1e049f3

Adds precondition for checking if user is not merging on itself

88ef74f

Swallow consistency error when decoding

3ab3546

Base error checking on return parameters as unsafe to check error / *…

e0ed31f

…error

Restore behaviour for nil tranformed objects without errors

2af299f

flovilmart added 23 commits January 30, 2018 09:42

nits: TODOs

bd3d622

Improve handling of failures for localId resolutiosn

cb97ec5

Report missing localId as soft errors

aa3f7c0

Refactor: Rename macros in PFPreconditon, more consistency on arguments

34f3b8f

Better error propagation in resolving localIds

5c3d70e

More tests for failures of localId resolutions

32886fc

Bump version to 1.16.1-alpha.1

e634afa

Bump podspec to 1.17.0-alpha.1

00e3393

Fix issue #916

9a237a1

- As per discussion over there, @nlutsenko and @richardjrossiii were A1 on the resolution Thanks @czgarrett for the fix Closes #931

Fixes #1184

c6660c2

- Prevent deadlock by copying estimatedData before traversal

Adds r/w accessors in swift for the PFACL

e88222c

fixes #1083

Adds CHANGELOG.md

59e7cc0

Bumps to 1.17.0-alpha.2

1855401

Fixes flaky test

3a71d2f

Provide more context when asserting OfflineStore objectId assignment …

7337f36

…issues

Bolts doesnt infer failed tasks with NSError as a return type, always…

a4ed6b7

… return failed tasks if needed

Bump to 1.17.0-alpha.3

b1addae

Releases on travis

20b8ddd

run pod trunk push with verbose to not stall travis

5d296a1

Review nits

155f30b

Do no track dimensions if they are nil

ea2fa1a

Bump version to 1.17.0-alpha.4

8a03368

Adds precondition to prevent crash

278ab82

flovilmart force-pushed the consistency-errors branch from d028cc4 to 278ab82 Compare January 30, 2018 14:44

flovilmart added 2 commits January 30, 2018 09:59

Fixes errors

6ede892

Bumps to 1.17.0-alpha.5

190bfc1

flovilmart merged commit 61f3320 into master Jan 30, 2018

flovilmart deleted the consistency-errors branch January 30, 2018 18:41

dplewis mentioned this pull request Jan 31, 2018

Full text search for Parse JS SDK and or Parse Cloud code parse-community/parse-server#4490

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Consistency errors #1237

Consistency errors #1237

flovilmart commented Jan 19, 2018 •

edited

Loading

flovilmart commented Jan 20, 2018

flovilmart commented Jan 20, 2018

flovilmart commented Jan 20, 2018

mt81 left a comment

mt81 Jan 22, 2018

mt81 Jan 22, 2018

flovilmart Jan 24, 2018

mt81 Jan 24, 2018

flovilmart Jan 24, 2018

mt81 Jan 24, 2018

mt81 Jan 24, 2018

mt81 Jan 24, 2018

mt81 Jan 24, 2018

mt81 Jan 24, 2018

mt81 Jan 24, 2018

mt81 Jan 24, 2018

flovilmart commented Jan 24, 2018

		@@ -66,6 +114,7 @@ do {\
		format:description, ##__VA_ARGS__]; \
		} while(0)

Consistency errors #1237

Consistency errors #1237

Conversation

flovilmart commented Jan 19, 2018 • edited Loading

flovilmart commented Jan 20, 2018

flovilmart commented Jan 20, 2018

flovilmart commented Jan 20, 2018

mt81 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flovilmart commented Jan 24, 2018

flovilmart commented Jan 19, 2018 •

edited

Loading