Skip to content

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

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.

Sign up for GitHub

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

Merged
Ldoppea merged 3 commits into from
Sep 9, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef893d5
feat: Forward operation in `StackLink.request()` when offline
Ldoppea May 31, 2024
f035d6c
feat: Forward operation in `StackLink.request()` on Network error
Ldoppea May 31, 2024
63ad248
docs: Update types and documentation
Ldoppea Jul 17, 2024
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
View file
Open in desktop
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
View file
Open in desktop
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)
Comment thread
paultranvan marked this conversation as resolved.
}
return await this.executeQuery(operation)
} catch (err) {
if (isReactNativeOfflineError(err)) {
return forward(operation)
Copy link
Copy Markdown
Contributor

@paultranvan paultranvan Jun 4, 2024

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
Copy Markdown
Contributor Author

@Ldoppea Ldoppea Jun 10, 2024

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
View file
Open in desktop
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
Copy Markdown
Contributor

@paultranvan paultranvan Aug 12, 2024

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
View file
Open in desktop
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
View file
Open in desktop
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