Allow error selector to change type

[sdesai]

This allows the errorSelector to change the $type of the leaf payload it gets in. So users can change certain errors, based on their payload, to an atom for example, if there is client side knowledge about values which can substitute for those errors.

We currently use this to fill out missing strings we request from the server, with reasonable client side defaults [ IIRC ]. We may still need to iterate on this, to see if we want to expand it's scope [ e.g. should the hook just become a general transformValueBeforeMerge hook - why does it just get errors as input. Are there use cases which warrant it getting non-errors also ].

@jhusain @lrowe @Blesh @forty2

[coveralls]

Coverage increased (+0.006%) to 91.934% when pulling 7b9d853 on allowErrorSelectorToChangeType into f41bb0a on master.

[sdesai]

Merging this in. Can apply feedback retroactively, if anyone has any.

[trxcllnt]

@sdesai fwiw, I had to change the error selector to accept the optimized (not requested) path in our @graphistry fork, because of an underlying optimistic concurrency issue that could lead to irreversible cache corruption.

When we send a request to the server, we have a set of requested and optimized paths. When the response arrives, we use the requested paths to merge the JSONGraph response into the cache. But if a reference that was used to build the requested optimized paths is changed (say, from lists[23], to lists[35]) while the request is in-flight, the JSONGraph merge with requested paths will fail catastrophically.

The JSONGraph merge with requested paths will follow the updated cache reference, but since the request was built with the older ref, the JSONGraph response doesn't have any branches corresponding to the new cache state, but also doesn't have any materialized values to validate the merge. When the merge reaches the new ref's location and finds an undefined in the message, it will blow away the node (and descendants) the new ref points to, and insert an undefined atom in its place.

This issue could manifest differently depending on whether the path of the new ref existed in the cache at merge time or not, but the end result was always merge failing and inserting an undefined atom somewhere it shouldn't.

I couldn't come up with many alternatives to using the optimized paths for the merge. Locks are a non-starter in our UI, and deep cloning cache fragments didn't seem like it'd scale, especially for batched (and now, streaming) requests. As far as I can tell, the only thing affected by this switch is the path passed to the error selector.

[sdesai]

Thanks for the input/context Paul. I appreciate the reach out. Just so I understand correctly:

1. There's a root bug fix to change merge, in general, to use optimized paths, as opposed to requested paths, to handle the scenario you mention - reference changing in flight.
2. A side effect of that root bug fix, is that it changes the input path passed to error selector from what used to be requested path, to the optimized path (because that's all we have at that point).

I'm not familiar enough with the design constraints (off the top of my head) to provide any useful feedback on whether or not there are alternate solutions for the root issue to consider/put on the table, vs. switching to optimized paths for merge, but I'll run it by the team when they have some free cycles, and if it seems like the best option on the table, the diff (thanks for that too) seems isolated enough to incorporate if the code bases haven't drifted in this area.

[trxcllnt]

@sdesai yep, that's the shape of things. Also, I've finished implementing the optimized cache search/recycled JSON with hash codes in the client, built streaming values into the client + router, and a built new DataSources (EventEmitter, Socket, PostMessage) to glue them together. It wasn't as easy as I'd expected, so I'd be happy to give an overview of the changes if those features are important to the team.

[sdesai]

Thanks for the offer. In general, I think it would be worth seeing how we can collaborate on some of this stuff (the performance areas are definitely a shared concern), but I can't guarantee resources currently to engage in design conversations, reviews, etc. at least in the short term. Happy to reach out if that flips, and we can discuss avenues to collaborate.