Treat server being down as client being offline #237
Conversation
jhellar
changed the title
|
@jhellar Is this another idea? |
|
@wtrocki basically this is just adding retry link (custom one - to allow force retry) and slightly refactored offlineLink, so that code can be reused between those two links. Will add description. |
jhellar
force-pushed
the
AEROGEAR-8447-2
branch
2 times, most recently
from
328c60f
to
f0b1a82
Compare
jhellar
force-pushed
the
AEROGEAR-8447-2
branch
from
02ecb57
to
6e3ae39
Compare
| @@ -25,26 +21,18 @@ export const createClient = async (userConfig?: DataSyncConfig): Promise<Voyager | |||
| const clientConfig = extractConfig(userConfig); | |||
| const { cache } = await buildCachePersistence(clientConfig); | |||
|
|
|||
| const httpLinks = await defaultHttpLinks(clientConfig); | |||
| let link = ApolloLink.from(httpLinks); | |||
There was a problem hiding this comment.
Yes. Thank you for taking that in. I wasn't really happy to see it here :)
| let links: ApolloLink[] = []; | ||
| if (config.networkStatus) { | ||
| const offlineLink = new OfflineLink({ | ||
| storage: config.storage, |
| const localDirectiveFilterLink = new LocalDirectiveFilterLink(); | ||
| export const defaultHttpLinks = async (config: DataSyncConfig): Promise<ApolloLink> => { | ||
| let links: ApolloLink[] = []; | ||
| if (config.networkStatus) { |
There was a problem hiding this comment.
IMHO this will be always non null. Is that because of typescript bragging about this being undefined?
There was a problem hiding this comment.
yes, this was because of typescript warning
There was a problem hiding this comment.
Yeah there are two patterns to sort this:
- Create separate interface with no optionals (too much IMHO)
- use
config.networkStatus as NetworkStatusto trick typescript. - if like here.
Let's leave it this way.
| * - operation was completed (which could result in ID updates, i.e. new | ||
| * operations ready to be forwarded - see OfflineQueue class) | ||
| */ | ||
| export class OfflineLink extends ApolloLink { |
There was a problem hiding this comment.
❤️ Man! Entire is very very clean and neatly drafted! Seriously good job.
| } | ||
|
|
||
| /** | ||
| * This makes sure that pending mutations are force retried also |
There was a problem hiding this comment.
[Question] pending mutations in this context means offline mutations in flight or every mutation?
There was a problem hiding this comment.
Here it should be all mutations in this link that are waiting to be retried
| import { RetriableOperation, shouldRetryFn } from "../offline/RetriableOperation"; | ||
|
|
||
| export interface RetryLinkOptions { | ||
| shouldRetry?: shouldRetryFn; |
There was a problem hiding this comment.
This is quite obvious but let's add just single node about this flag.
|
@jhellar I haven't seen such good code in very long time. Everything is so neatly drafted. Before I explode in all that excitement I will need to release development version of this and try test it with the app. |
wtrocki
requested review from
StephenCoady,
danielpassos,
secondsun and
darahayes
|
As @jhellar is on holidays I'm taking over this task |
wtrocki
changed the title
Description
This is implemented with custom
RetryLink[1]. It has a queue for all incoming operations. When there is a new operation, it will try to forward it. On network error it will schedule next try. If there is operation with response from server, all operations in queue are force retried.When creating
RetryLinkuser can provideshouldRetryfunction to cancel retry if needed.[1] https://github.com/aerogear/aerogear-js-sdk/pull/237/files#diff-46acc41354166331ff353344d6065e85
Checklist
integration testsIntegration tests moved to separate jira - https://issues.jboss.org/browse/AEROGEAR-8521