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

Fix: Allow implementers of view-controller to set if it's connected to the internet, and reject requests when isConnectedToInternet is false #673

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
13 changes: 4 additions & 9 deletions packages/@magic-sdk/provider/src/core/view-controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ async function persistMagicEventRefreshToken(event: MagicMessageEvent) {
}

export abstract class ViewController {
private isReadyForRequest = false;
public checkIsReadyForRequest: Promise<void>;
protected readonly messageHandlers = new Set<(event: MagicMessageEvent) => any>();
protected isConnectedToInternet = true;

/**
* Create an instance of `ViewController`
Expand Down Expand Up @@ -132,17 +132,13 @@ export abstract class ViewController {
payload: JsonRpcRequestPayload | JsonRpcRequestPayload[],
): Promise<JsonRpcResponse<ResultType> | JsonRpcResponse<ResultType>[]> {
return createPromise(async (resolve, reject) => {
if (SDKEnvironment.platform !== 'react-native') {
await this.checkIsReadyForRequest;
} else if (!this.isReadyForRequest) {
// On a mobile environment, `this.checkIsReadyForRequest` never resolves
// if the app was initially opened without internet connection. That is
// why we reject the promise without waiting and just let them call it
// again when internet connection is re-established.
if (!this.isConnectedToInternet) {
const error = createModalNotReadyError();
reject(error);
}

await this.checkIsReadyForRequest;

const batchData: JsonRpcResponse[] = [];
const batchIds = Array.isArray(payload) ? payload.map((p) => p.id) : [];
const msg = await createMagicRequest(`${msgType}-${this.parameters}`, payload);
Expand Down Expand Up @@ -207,7 +203,6 @@ export abstract class ViewController {
return new Promise<void>((resolve) => {
this.on(MagicIncomingWindowMessage.MAGIC_OVERLAY_READY, () => {
resolve();
this.isReadyForRequest = true;
});
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,15 @@ test('Sends payload and stores rt if response event contains rt', async () => {
expect(FAKE_STORE.rt).toEqual(FAKE_RT);
});

test('does not wait for ready and throws error when platform is react-native', async () => {
SDKEnvironment.platform = 'react-native';
test('throws MODAL_NOT_READY error when not connected to the internet', async () => {
const eventWithRt = { data: { ...responseEvent().data } };
const viewController = createViewController('asdf');
const { handlerSpy, onSpy } = stubViewController(viewController, [
[MagicIncomingWindowMessage.MAGIC_HANDLE_RESPONSE, eventWithRt],
]);
viewController.checkIsReadyForRequest = new Promise(() => null);

// @ts-ignore protected variable
viewController.isConnectedToInternet = false;

const payload = requestPayload();

Expand All @@ -217,9 +218,6 @@ test('does not wait for ready and throws error when platform is react-native', a
} catch (e) {
expect(e).toEqual(createModalNotReadyError());
}
expect(createJwtStub).not.toHaveBeenCalledWith();
expect(onSpy.mock.calls[0][0]).toEqual(MagicIncomingWindowMessage.MAGIC_HANDLE_RESPONSE);
expect(handlerSpy).not.toHaveBeenCalled();
});

test('does not call web crypto api if platform is not web', async () => {
Expand All @@ -231,9 +229,6 @@ test('does not call web crypto api if platform is not web', async () => {
]);
const payload = requestPayload();

// @ts-ignore isReadyForRequest is private
viewController.isReadyForRequest = true;

const response = await viewController.post(MagicOutgoingWindowMessage.MAGIC_HANDLE_REQUEST, payload);

expect(createJwtStub).not.toHaveBeenCalledWith();
Expand Down
2 changes: 1 addition & 1 deletion packages/@magic-sdk/react-native-bare/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@magic-sdk/react-native-bare",
"version": "22.3.0",
"version": "22.3.1",
"description": "Passwordless authentication for React Native (Bare).",
"author": "Magic Labs <[email protected]> (https://magic.link/)",
"license": "MIT",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { useState, useCallback, useMemo } from 'react';
import React, { useState, useCallback, useMemo, useEffect } from 'react';
import { Linking, StyleSheet, View } from 'react-native';
import { WebView } from 'react-native-webview';
import { SafeAreaView } from 'react-native-safe-area-context';
import { ViewController, createModalNotReadyError } from '@magic-sdk/provider';
import { MagicMessageEvent } from '@magic-sdk/types';
import { isTypedArray } from 'lodash';
import Global = NodeJS.Global;
import { useInternetConnection } from './hooks';

const MAGIC_PAYLOAD_FLAG_TYPED_ARRAY = 'MAGIC_PAYLOAD_FLAG_TYPED_ARRAY';
const OPEN_IN_DEVICE_BROWSER = 'open_in_device_browser';
Expand Down Expand Up @@ -79,6 +80,11 @@ export class ReactNativeWebViewController extends ViewController {
/* istanbul ignore next */
public Relayer: React.FC<{ backgroundColor?: string }> = (backgroundColor) => {
const [show, setShow] = useState(false);
const isConnected = useInternetConnection();

useEffect(() => {
this.isConnectedToInternet = isConnected;
}, [isConnected]);

/**
* Saves a reference to the underlying `<WebView>` node so we can interact
Expand Down
2 changes: 1 addition & 1 deletion packages/@magic-sdk/react-native-expo/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@magic-sdk/react-native-expo",
"version": "22.3.0",
"version": "22.3.1",
"description": "Passwordless authentication for React Native (Expo).",
"author": "Magic Labs <[email protected]> (https://magic.link/)",
"license": "MIT",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, { useState, useCallback, useMemo } from 'react';
import React, { useState, useCallback, useMemo, useEffect } from 'react';
import { Linking, StyleSheet, View } from 'react-native';
import { WebView } from 'react-native-webview';
import { SafeAreaView } from 'react-native-safe-area-context';
import { ViewController, createModalNotReadyError } from '@magic-sdk/provider';
import { MagicMessageEvent } from '@magic-sdk/types';
import { isTypedArray } from 'lodash';
import Global = NodeJS.Global;
import { useInternetConnection } from './hooks';

const MAGIC_PAYLOAD_FLAG_TYPED_ARRAY = 'MAGIC_PAYLOAD_FLAG_TYPED_ARRAY';
const OPEN_IN_DEVICE_BROWSER = 'open_in_device_browser';
Expand Down Expand Up @@ -79,6 +80,11 @@ export class ReactNativeWebViewController extends ViewController {
/* istanbul ignore next */
public Relayer: React.FC<{ backgroundColor?: string }> = ({ backgroundColor }) => {
const [show, setShow] = useState(false);
const isConnected = useInternetConnection();

useEffect(() => {
this.isConnectedToInternet = isConnected;
}, [isConnected]);

/**
* Saves a reference to the underlying `<WebView>` node so we can interact
Expand Down