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

Transformable changes: crash #330

Closed
jphribovsek opened this issue Aug 13, 2014 · 9 comments
Closed

Transformable changes: crash #330

jphribovsek opened this issue Aug 13, 2014 · 9 comments
Labels

Comments

@jphribovsek
Copy link

We are inserting some data server side in one of our bucket. One of the bucket attributes is a transformable (bas64 string on the server).
When we modify that transformable server side, the change that is sent to the iOS client is a diff of the base64 string, similar as updating a regular string.
However, Simperium SDK attempts to apply the reverse transformer on the diff string https://github.com/Simperium/simperium-ios/blob/develop/Simperium/SPDiffer.m#L201
which gives a null value as the transformer can't decode a change string. The app eventually crashes.

Based on the SDK, it appears that in the case of base64 strings, the SDK expects to receive the entire string at every change, and not a change string.

Is there anything we can do server side to make sure the entire object is sent?

@jleandroperez
Copy link
Contributor

hello @jphribovsek!

May i ask you what you mean by apply the reverse transformer on the diff string? (the linked code just shows what is expected to get the delta string).

Would it be possible to get a crashlog? .

Regarding sending entire objects instead of deltas, there is a way, by means of backend schemas. /cc @fredrocious

Thanks!

@jleandroperez jleandroperez added this to the v0.6.8 milestone Aug 13, 2014
@jphribovsek
Copy link
Author

in SPDiffer, if you look at the call id otherValue = [member getValueFromDictionary:change key:OP_VALUE object:object];
if member is a SPMemberBase64 then getValueFromDictionary converts that change string to NSData (nil since it's not a full base64 string), and use the reverse transformer to get the decoded object https://github.com/Simperium/simperium-ios/blob/develop/Simperium/SPMemberBase64.m#L39-L42

therefore getValueFromDictionary returns nil, we get the "Simperium error: member %@ from diff wasn't in change" error message

the app eventually crashes:

2014-08-13 14:58:36.734 xxxxxxxxxx[57589:1303] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[NSNull length]: unrecognized selector sent to instance 0x767b068'
*** First throw call stack:
(
0 CoreFoundation 0x0752f1e4 exceptionPreprocess + 180
1 libobjc.A.dylib 0x071f88e5 objc_exception_throw + 44
2 CoreFoundation 0x075cc243 -[NSObject(NSObject) doesNotRecognizeSelector:] + 275
3 CoreFoundation 0x0751f50b __forwarding
+ 1019
4 CoreFoundation 0x0751f0ee _CF_forwarding_prep_0 + 14
5 xxxxxxxxxx 0x00fcde35 -[DiffMatchPatch diff_fromDeltaWithText:andDelta:error:] + 3717
6 xxxxxxxxxx 0x00ffaf8a -[SPMemberText transform:otherValue:oldValue:error:] + 282
7 xxxxxxxxxx 0x010201a4 -[SPDiffer transform:diff:oldDiff:oldGhost:error:] + 2164
8 xxxxxxxxxx 0x00fec647 -[SPChangeProcessor processRemoteModifyWithKey:bucket:change:acknowledged:clientMatches:error:] + 6295
9 xxxxxxxxxx 0x00fee7ad -[SPChangeProcessor processRemoteChange:bucket:error:] + 3309
10 xxxxxxxxxx 0x00fef718 -[SPChangeProcessor processRemoteChanges:bucket:errorHandler:] + 1672
11 xxxxxxxxxx 0x0103ce21 __49-[SPWebSocketChannel processBatchChanges:bucket:]_block_invoke + 305
12 libdispatch.dylib 0x084137b8 _dispatch_call_block_and_release + 15
13 libdispatch.dylib 0x084284d0 _dispatch_client_callout + 14
14 libdispatch.dylib 0x08416047 _dispatch_queue_drain + 452
15 libdispatch.dylib 0x08415e42 _dispatch_queue_invoke + 128
16 libdispatch.dylib 0x08416de2 _dispatch_root_queue_drain + 78
17 libdispatch.dylib 0x08417127 _dispatch_worker_thread2 + 39
18 libsystem_pthread.dylib 0x08753606 _pthread_wqthread + 752
19 libsystem_pthread.dylib 0x087512de start_wqthread + 30
)

@jphribovsek
Copy link
Author

if you look at applyDiff:otherValue:error for SPMemberBase64.m, you can see that otherValue is expected to be a complete object, and not a delta string.
Therefore when id otherValue = [member getValueFromDictionary:change key:OP_VALUE object:object]; is called in - (BOOL)applyGhostDiffFromDictionary:(NSDictionary *)diff toObject:(id<SPDiffable>)object error:(NSError **)error you would expect change to be the complete object base64 representation, not the diff between the previous and current base64 string. However, this is the kind of data that is coming back when changing a transformable server side (delta string):

{
    extendedInformation =     {
        o = d;
        v = "=330\t-2\t+EA\t=336\t-4\t+dGVzdHp6";
    };
}

@jphribovsek
Copy link
Author

Note that there are two different issues here:

  • as previously described, the transformable changes are not processed because the base64 diff fails
  • the crash actually comes from a different problem it looks like, we are saving some attributes as 'null' server side, which is what causes the crash

@danieljimenez
Copy link
Contributor

We would be interested in a way to disable diffs via schema for the base64 data we are sending.

@jleandroperez
Copy link
Contributor

Hello there @danieljimenez + @jphribovsek,

Please, try setting the following schema attributes, in order disable diffs:

"schema": {
    "attributes": {
      "body": {
        "otype": "replace"
      },
      "subject": {
        "otype": "replace"
      }
    }
  }

Note:
In this snippet, you would be toggling replace OP on for both subject + body fields.

Please, bear in mind that by doing this, you would be disabling diff'ing for changes flowing from the backend to the clients. However, i'm afraid that, at least for now, the client lib doesn't handle schema options. (Which means, in other words, that any client uploading a change is likely to experience this scenario).

@jphribovsek thanks for the details, much appreciated!

@jleandroperez jleandroperez modified the milestones: v0.6.8, v0.6.9 Aug 27, 2014
@jleandroperez jleandroperez modified the milestone: v0.6.9 Sep 9, 2014
@jleandroperez
Copy link
Contributor

Closed via #353

@jphribovsek
Copy link
Author

Jorge, is the server side workaround considered a fix for this issue? I was still hoping for an SDK fix; supporting the diff of base64 strings would be nice to save bandwidth

@jleandroperez
Copy link
Contributor

@jphribovsek We'll be implementing the schema setting client side. At least for now, there is no support for diffing base64 strings.

Thanks JP!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants