Skip to content

Commit

Permalink
feat: cleanup leaking console logs (#907)
Browse files Browse the repository at this point in the history
feat: wip
  • Loading branch information
abretonc7s authored Jun 21, 2024
1 parent 5276cd6 commit 53a7f5a
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export function createChannel(instance: SocketService) {
throw new Error(`socket already connected`);
}

console.log(`create channel`, instance.state.socket);
instance.state.socket?.connect();
instance.state.manualDisconnect = false;
instance.state.isOriginator = true;
Expand Down
1 change: 1 addition & 0 deletions packages/sdk/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export const RPC_METHODS = {
METAMASK_BATCH: 'metamask_batch',
PERSONAL_SIGN: 'personal_sign',
WALLET_REQUESTPERMISSIONS: 'wallet_requestPermissions',
WALLET_REVOKEPERMISSIONS: 'wallet_revokePermissions',
WALLET_GETPERMISSIONS: 'wallet_getPermissions',
WALLET_WATCHASSET: 'wallet_watchAsset',
WALLET_ADDETHEREUMCHAIN: 'wallet_addEthereumChain',
Expand Down
5 changes: 2 additions & 3 deletions packages/sdk/src/sdk.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,12 +330,11 @@ describe('MetaMaskSDK', () => {
});

describe('Deprecated Methods', () => {
it('should log warning and call terminate when disconnect is called', () => {
it('should log warning and call terminate when disconnect is called', async () => {
const consoleWarnSpy = jest.spyOn(console, 'warn');
const terminateSpy = jest.spyOn(sdk, 'terminate');

sdk.disconnect();

await sdk.disconnect();
expect(consoleWarnSpy).toHaveBeenCalledWith(
'MetaMaskSDK.disconnect() is deprecated, use terminate()',
);
Expand Down
2 changes: 1 addition & 1 deletion packages/sdk/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ export class MetaMaskSDK extends EventEmitter2 {
*/
disconnect() {
console.warn(`MetaMaskSDK.disconnect() is deprecated, use terminate()`);
this.terminate();
return this.terminate();
}

isAuthorized() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ describe('terminate', () => {
describe('when in MetaMask Mobile WebView', () => {
beforeEach(() => mockIsMetaMaskMobileWebView.mockReturnValue(true));

it('should do nothing', () => {
terminate(instance);
it('should do nothing', async () => {
await terminate(instance);
expect(mockDisconnect).not.toHaveBeenCalled();
expect(mockEmit).not.toHaveBeenCalled();
});
Expand All @@ -61,35 +61,35 @@ describe('terminate', () => {

// TODO re-enable once we can mock window object or external storage provider
// eslint-disable-next-line jest/no-disabled-tests
it.skip('should remove extension provider', () => {
terminate(instance);
it.skip('should remove extension provider', async () => {
await terminate(instance);
expect(localStorageMock.removeItem).toHaveBeenCalledWith(
STORAGE_PROVIDER_TYPE,
);
});

it('should switch back to default provider', () => {
terminate(instance);
it('should switch back to default provider', async () => {
await terminate(instance);
expect(instance.activeProvider).toBe(instance.sdkProvider);
expect((global.window as any).ethereum).toBe(instance.activeProvider);
});

it('should set extensionActive to false', () => {
terminate(instance);
it('should set extensionActive to false', async () => {
await terminate(instance);
expect(instance.extensionActive).toBe(false);
});

it('should emit PROVIDER_UPDATE with TERMINATE', () => {
terminate(instance);
it('should emit PROVIDER_UPDATE with TERMINATE', async () => {
await terminate(instance);
expect(mockEmit).toHaveBeenCalledWith(
EventType.PROVIDER_UPDATE,
PROVIDER_UPDATE_TYPE.TERMINATE,
);
});

it('should not switch providers if extensionOnly option is true', () => {
it('should not switch providers if extensionOnly option is true', async () => {
instance.options.extensionOnly = true;
terminate(instance);
await terminate(instance);
expect(mockEmit).not.toHaveBeenCalled();
});
});
Expand All @@ -99,24 +99,24 @@ describe('terminate', () => {
instance.extensionActive = false;
});

it('should emit PROVIDER_UPDATE with TERMINATE', () => {
terminate(instance);
it('should emit PROVIDER_UPDATE with TERMINATE', async () => {
await terminate(instance);
expect(mockEmit).toHaveBeenCalledWith(
EventType.PROVIDER_UPDATE,
PROVIDER_UPDATE_TYPE.TERMINATE,
);
});

it('should log debug messages', () => {
terminate(instance);
it('should log debug messages', async () => {
await terminate(instance);

expect(spyLogger).toHaveBeenCalledWith(
`[MetaMaskSDK: terminate()] remoteConnection=${instance.remoteConnection}`,
);
});

it('should disconnect remote connection', () => {
terminate(instance);
it('should disconnect remote connection', async () => {
await terminate(instance);
expect(mockDisconnect).toHaveBeenCalledWith({
terminate: true,
sendMessage: true,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { EventType } from '@metamask/sdk-communication-layer';
import { logger } from '../../../utils/logger';
import {
RPC_METHODS,
STORAGE_DAPP_CHAINID,
STORAGE_DAPP_SELECTED_ADDRESS,
STORAGE_PROVIDER_TYPE,
} from '../../../config';
import { MetaMaskSDK } from '../../../sdk';
import { PROVIDER_UPDATE_TYPE } from '../../../types/ProviderUpdateType';
import { logger } from '../../../utils/logger';

const hasLocalStoage = typeof window !== 'undefined' && window.localStorage;

Expand All @@ -21,7 +22,7 @@ const hasLocalStoage = typeof window !== 'undefined' && window.localStorage;
* @returns void
* @emits EventType.PROVIDER_UPDATE with payload PROVIDER_UPDATE_TYPE.TERMINATE when the provider is updated.
*/
export function terminate(instance: MetaMaskSDK) {
export async function terminate(instance: MetaMaskSDK) {
// nothing to do on inapp browser.
if (instance.platformManager?.isMetaMaskMobileWebView()) {
return;
Expand All @@ -36,12 +37,23 @@ export function terminate(instance: MetaMaskSDK) {
// check if connected with extension provider
// if it is, disconnect from it and switch back to injected provider
if (instance.extensionActive) {
try {
// Revoke permissions
await instance.activeProvider?.request({
method: RPC_METHODS.WALLET_REVOKEPERMISSIONS,
params: [{ eth_accounts: {} }],
});
} catch (error) {
logger(`[MetaMaskSDK: terminate()] error revoking permissions`, error);
}

if (instance.options.extensionOnly) {
logger(
`[MetaMaskSDK: terminate()] extensionOnly --- prevent switching providers`,
);
return;
}

// Re-use default extension provider as default
instance.activeProvider = instance.sdkProvider;
window.ethereum = instance.activeProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('showInstallModal', () => {
},
getMetaMaskInstaller: jest.fn(),
sdk: {
terminate: jest.fn(),
terminate: jest.fn().mockResolvedValue(undefined),
},
connectWithExtensionProvider: jest.fn(),
} as unknown as RemoteConnectionProps;
Expand All @@ -58,15 +58,15 @@ describe('showInstallModal', () => {
expect(mockInstallModalMount).toHaveBeenCalledTimes(1);
});

it('should terminate the connection and possibly log the termination', () => {
it('should terminate the connection and possibly log the termination', async () => {
const link = 'http://example.com/terminate';

showInstallModal(state, options, link);

const terminateCall = mockModalsInstall.mock.calls[0][0]
.terminate as () => void;
.terminate as () => Promise<void>;

terminateCall();
await terminateCall();

expect(spyLogger).toHaveBeenCalledWith(
'[RemoteConnection: showInstallModal() => terminate()] terminate connection',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ export function showInstallModal(
);

// Terminate with specific error code
options.sdk.terminate();
options.sdk.terminate().catch((err) => {
console.warn(`[MMSDK] failed to terminate connection`, err);
});
},
debug: state.developerMode,
connectWithExtension: () => {
Expand Down

0 comments on commit 53a7f5a

Please sign in to comment.