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

feat: Forward operation in StackLink.request() when offline #1487

Merged
merged 3 commits into from
Sep 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions docs/api/cozy-client/classes/StackLink.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Transfers queries and mutations to a remote stack
| :------ | :------ | :------ |
| `[options]` | `Object` | Options |
| `[options].client` | `any` | - |
| `[options].platform` | `any` | - |
| `[options].stackClient` | `any` | - |

*Overrides*
Expand All @@ -30,17 +31,27 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:62](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L62)
[packages/cozy-client/src/StackLink.js:64](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L64)

## Properties

### isOnline

• **isOnline**: `any`

*Defined in*

[packages/cozy-client/src/StackLink.js:72](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L72)

***

### stackClient

• **stackClient**: `any`

*Defined in*

[packages/cozy-client/src/StackLink.js:69](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L69)
[packages/cozy-client/src/StackLink.js:71](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L71)

## Methods

Expand All @@ -62,7 +73,7 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:118](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L118)
[packages/cozy-client/src/StackLink.js:132](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L132)

***

Expand All @@ -82,7 +93,7 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:95](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L95)
[packages/cozy-client/src/StackLink.js:109](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L109)

***

Expand All @@ -107,7 +118,7 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:87](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L87)
[packages/cozy-client/src/StackLink.js:101](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L101)

***

Expand All @@ -127,7 +138,7 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:72](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L72)
[packages/cozy-client/src/StackLink.js:75](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L75)

***

Expand All @@ -153,7 +164,7 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:80](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L80)
[packages/cozy-client/src/StackLink.js:83](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L83)

***

Expand All @@ -167,4 +178,4 @@ Transfers queries and mutations to a remote stack

*Defined in*

[packages/cozy-client/src/StackLink.js:76](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L76)
[packages/cozy-client/src/StackLink.js:79](https://github.com/cozy/cozy-client/blob/master/packages/cozy-client/src/StackLink.js#L79)
24 changes: 19 additions & 5 deletions packages/cozy-client/src/StackLink.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import CozyLink from './CozyLink'
import { DOCTYPE_FILES } from './const'
import { BulkEditError } from './errors'
import logger from './logger'
import { isReactNativeOfflineError } from './utils'

/**
*
Expand Down Expand Up @@ -58,15 +59,17 @@ export default class StackLink extends CozyLink {
* @param {object} [options] - Options
* @param {object} [options.stackClient] - A StackClient
* @param {object} [options.client] - A StackClient (deprecated)
* @param {import('cozy-pouch-link/dist/types').LinkPlatform} [options.platform] Platform specific adapters and methods
*/
constructor({ client, stackClient } = {}) {
constructor({ client, stackClient, platform } = {}) {
super()
if (client) {
logger.warn(
'Using options.client is deprecated, prefer options.stackClient'
)
}
this.stackClient = stackClient || client
this.isOnline = platform?.isOnline
}

registerClient(client) {
Expand All @@ -77,11 +80,22 @@ export default class StackLink extends CozyLink {
this.stackClient = null
}

request(operation, result, forward) {
if (operation.mutationType) {
return this.executeMutation(operation, result, forward)
async request(operation, result, forward) {
if (this.isOnline && !(await this.isOnline())) {
return forward(operation)
}

try {
if (operation.mutationType) {
return await this.executeMutation(operation, result, forward)
paultranvan marked this conversation as resolved.
Show resolved Hide resolved
}
return await this.executeQuery(operation)
} catch (err) {
if (isReactNativeOfflineError(err)) {
return forward(operation)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is a fallback in case the this.isOnline() was not defined or wrongly returned true? I see the value, but I'm wondering whether it is good to have this kind of logic here.
What if someone has a Cozy react-native app with only the StackLink defined? Here, it will forward the operation and the app will never receive the error

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is a fallback in case the this.isOnline() was not defined or wrongly returned true?

Yes, this is mainly because when a device loose the Internet connection, it can take a few seconds before the offline status is triggered. This often happens when you are connected to an Internet box through Wifi. If you unplug the internet connection, then your computer/phones will still displayed the "online" icon for a few seconds.

What if someone has a Cozy react-native app with only the StackLink defined? Here, it will forward the operation and the app will never receive the error

Good catch. But I'm not sure that we can check if any next link exist. I'll investigate how to do this correctly.

}
throw err
}
return this.executeQuery(operation)
}

async persistData(data, forward) {
Expand Down
12 changes: 12 additions & 0 deletions packages/cozy-client/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,15 @@ export const hasQueriesBeenLoaded = queriesResults => {
hasQueryBeenLoaded(queryResult)
)
}

/**
* Check is the error is about ReactNative not having access to internet
*
* @param {Error} err - The error to check
* @returns {boolean} True if the error is a network error, otherwise false
*/
export const isReactNativeOfflineError = err => {
// This error message is specific to ReactNative
// Network errors on a browser would produce another error.message
return err.message === 'Network request failed'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how stable this message is, and if it could change in future RN upgrades...

}
5 changes: 4 additions & 1 deletion packages/cozy-client/types/StackLink.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ export default class StackLink extends CozyLink {
* @param {object} [options] - Options
* @param {object} [options.stackClient] - A StackClient
* @param {object} [options.client] - A StackClient (deprecated)
* @param {import('cozy-pouch-link/dist/types').LinkPlatform} [options.platform] Platform specific adapters and methods
*/
constructor({ client, stackClient }?: {
constructor({ client, stackClient, platform }?: {
stackClient: object;
client: object;
platform: import('cozy-pouch-link/dist/types').LinkPlatform;
});
stackClient: any;
isOnline: any;
registerClient(client: any): void;
reset(): void;
/**
Expand Down
1 change: 1 addition & 0 deletions packages/cozy-client/types/utils.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export function isQueryLoading(col: any): boolean;
export function hasQueryBeenLoaded(col: any): any;
export function isQueriesLoading(queriesResults: any): boolean;
export function hasQueriesBeenLoaded(queriesResults: any): boolean;
export function isReactNativeOfflineError(err: Error): boolean;
export type CancelablePromise = Promise<any>;
/**
* @typedef {Promise} CancelablePromise
Expand Down