Cache load errors cause deadlock in write transaction #365
Conversation
|
One travis failure, which passes on my local. Unsure why it's breaking. Can someone kick it? I suspect it's a timing-related failure. |
| let query = HeroAndFriendsNamesQuery() | ||
| load(query: query) { (result, error) in | ||
| XCTAssertNil(result) | ||
| XCTAssertNotNil(error) |
There was a problem hiding this comment.
Updating this to
XCTAssertTrue(error is GraphQLResultError)
XCTAssertTrue((error as! GraphQLResultError).underlying is JSONDecodingError)provides a bit more confidence about what is happening here.
There was a problem hiding this comment.
I'd use guard let instead of force unwrapping because force unwrapping crashes the whole test suite instead of just failing the test, but I agree here
There was a problem hiding this comment.
@ashiemke No idea why that one test failed, rerunning the whole suite. If you can rebase this on master so we can run it against Xcode 10, that'd be lovely - this seems like it could help.
|
Yeah you'll definitely need to rebase since the project is now using Swift 5 - went from only one test failing to all of them failing 🤦♀️ |
|
@ashiemke are you interested in continuing this PR? |
|
@ashiemke Since I haven't heard back from ya in a while, I'm going to take this over so we can try to get it merged. |
|
Closing this one out, please follow #763 for an attempted resolution here. |
Reject batch loader loads if a loader fails to allow exceptions to cascade through promise chain. Previous behavior (run test without code) causes the load to never complete, and more importantly, causes the store to hold a read lock indefinitely, causing a deadlock when a write is attempted later. In practice, this is every time a fetch occurs with default cache policy.
The root of the issue is a non-normalized dictionary which is stored in the cache. In our schema, we have some JSON blobs that come from an external service which are represented as a JSON custom scalar. Storing these in the cache worked fine, but fetching these types caused attempts to read from cache to never return, and other strange behavior due to writes also failing.
Solution was pretty simple, just needed to pass the failures in the batch loader to the individual load promises so they can be picked up and forwarded on the chain. Since these are wrapped in a
whenAll, only the firstrejectmatters.