Skip to content

Commit

Permalink
fix: Remove invalid providerConfig ids
Browse files Browse the repository at this point in the history
Remove invalid `providerConfig.id` state. This was one possible
explanation for some Sentry errors that we have been encountering in
production. The diagnostic state attached to the error was not
sufficient to confirm that this was the cause.

Relates to #26309
  • Loading branch information
Gudahtt committed Aug 2, 2024
1 parent 69e57d8 commit 9dfec33
Show file tree
Hide file tree
Showing 2 changed files with 182 additions and 1 deletion.
140 changes: 139 additions & 1 deletion app/scripts/migrations/120.2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,72 @@ describe('migration #120.2', () => {
);
});

it('does nothing if obsolete properties are not set', async () => {
it('captures an error and leaves state unchanged if providerConfig state is corrupted', async () => {
const oldState = {
NetworkController: {
providerConfig: 'invalid',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
new Error(
`Migration ${version}: Invalid NetworkController providerConfig state of type 'string'`,
),
);
});

it('captures an error and leaves state unchanged if networkConfigurations state is corrupted', async () => {
const oldState = {
NetworkController: {
networkConfigurations: 'invalid',
providerConfig: {},
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
expect(sentryCaptureExceptionMock).toHaveBeenCalledWith(
new Error(
`Migration ${version}: Invalid NetworkController networkConfigurations state of type 'string'`,
),
);
});

it('does nothing if obsolete properties and providerConfig id are not set', async () => {
const oldState = {
NetworkController: {
providerConfig: {},
selectedNetworkClientId: 'example',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data).toEqual(oldState);
});

it('does not remove a valid providerConfig id', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
'valid-id': {},
},
providerConfig: {
id: 'valid-id',
},
selectedNetworkClientId: 'example',
},
};
Expand Down Expand Up @@ -289,6 +352,81 @@ describe('migration #120.2', () => {
});
});

it('removes providerConfig id if network configuration is missing', async () => {
const oldState = {
NetworkController: {
providerConfig: {
id: 'invalid-id',
},
selectedNetworkClientId: 'example',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data.NetworkController).toEqual({
providerConfig: {},
selectedNetworkClientId: 'example',
});
});

it('removes providerConfig id that does not match any network configuration', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
'valid-id': {},
},
providerConfig: {
id: 'invalid-id',
},
selectedNetworkClientId: 'example',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data.NetworkController).toEqual({
networkConfigurations: {
'valid-id': {},
},
providerConfig: {},
selectedNetworkClientId: 'example',
});
});

it('removes providerConfig id with an invalid type', async () => {
const oldState = {
NetworkController: {
networkConfigurations: {
'123': {},
},
providerConfig: {
id: 123,
},
selectedNetworkClientId: 'example',
},
};

const transformedState = await migrate({
meta: { version: oldVersion },
data: cloneDeep(oldState),
});

expect(transformedState.data.NetworkController).toEqual({
networkConfigurations: {
'123': {},
},
providerConfig: {},
selectedNetworkClientId: 'example',
});
});

it('still migrates NetworkController state if other controllers have invalid state', async () => {
const oldState = {
NetworkController: {
Expand Down
43 changes: 43 additions & 0 deletions app/scripts/migrations/120.2.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { cloneDeep } from 'lodash';
import { hasProperty, isObject } from '@metamask/utils';
import log from 'loglevel';

type VersionedData = {
meta: { version: number };
Expand Down Expand Up @@ -103,6 +104,48 @@ function removeObsoleteNetworkControllerState(

const networkControllerState = state.NetworkController;

// Check for invalid `providerConfig.id`, and remove if found
if (hasProperty(networkControllerState, 'providerConfig')) {
if (!isObject(networkControllerState.providerConfig)) {
global.sentry.captureException(
new Error(
`Migration ${version}: Invalid NetworkController providerConfig state of type '${typeof state
.NetworkController.providerConfig}'`,
),
);
return;
}
const { providerConfig } = networkControllerState;

const validNetworkConfigurationIds = [];
if (hasProperty(networkControllerState, 'networkConfigurations')) {
if (!isObject(networkControllerState.networkConfigurations)) {
global.sentry.captureException(
new Error(
`Migration ${version}: Invalid NetworkController networkConfigurations state of type '${typeof networkControllerState.networkConfigurations}'`,
),
);
return;
}

validNetworkConfigurationIds.push(
...Object.keys(networkControllerState.networkConfigurations),
);
}

if (hasProperty(providerConfig, 'id')) {
if (
typeof providerConfig.id !== 'string' ||
!validNetworkConfigurationIds.includes(providerConfig.id)
) {
log.warn(
`Migration ${version}: Removing invalid provider id ${providerConfig.id}`,
);
delete providerConfig.id;
}
}
}

delete networkControllerState.networkDetails;
delete networkControllerState.networkId;
delete networkControllerState.networkStatus;
Expand Down

0 comments on commit 9dfec33

Please sign in to comment.