fix: remove namespace from storage on auto connect failure #988
Conversation
RyukTheCoder
changed the base branch from
next
to
fix/rf-2098-update-connected-namespaces-in-storage-on-switch-account
RyukTheCoder
changed the title
RyukTheCoder
changed the title
|
|
||
| return async () => await namespace.connect(chain); | ||
| }); | ||
| return async () => await namespace.connect(chain); |
There was a problem hiding this comment.
since you are making a list of failed promises using instanceof Error, ensure you will throw an error in any cases by having something like this:
.catch(e => {
if(e instanceof Error){
throw e;
}
throw new Error(e)
})
There was a problem hiding this comment.
write this assumption as a comment on top of it also.
| ); | ||
| } | ||
|
|
||
| const atLeastOneNamespaceConnectedSuccessfully = successNamespaces.length > 0; |
There was a problem hiding this comment.
you can remove successNamespaces and consoder it as:
| const atLeastOneNamespaceConnectedSuccessfully = successNamespaces.length > 0; | |
| const atLeastOneNamespaceConnectedSuccessfully = connectNamespacesResult.length - failedNamespaces.length > 0; |
I'm suggesting this since you only need the length, and not the input itselt.
| throw new Error(`No namespace connected for ${type}`); | ||
| } | ||
|
|
||
| return connectNamespacesResult.filter((result) => !(result instanceof Error)); |
There was a problem hiding this comment.
| return connectNamespacesResult.filter((result) => !(result instanceof Error)); | |
| const successfulResult = connectNamespacesResult.filter((result) => !(result instanceof Error)); | |
| return successfulResult |
| @@ -161,26 +190,15 @@ export async function autoConnect(deps: { | |||
| providerName, | |||
| }).catch((e) => { | |||
| walletsToRemoveFromPersistance.push(providerName); | |||
| throw e; | |||
| console.warn(e); | |||
| }) | |||
| ); | |||
| }); | |||
|
|
|||
| await Promise.allSettled(eagerConnectQueue); | |||
There was a problem hiding this comment.
with your change on remving throw, the promise never fails, so you can change this to Promise.all i guess
|
|
||
| return [...previousResults, taskResult]; | ||
| return [...previousResults, taskResult]; | ||
| } catch (error) { |
There was a problem hiding this comment.
you've change the behavior and it doesn't reflect in the name. you can create a new function called sequentiollyRunWithoutFailing
Summary
Remove namespace from storage on auto connect failure.
Fixes # (RF-2118)
How did you test this change?
Tested this change by changin phantom account to an account containg only Solana address and setting
{"phantom":[{"namespace":"Solana"},{"namespace":"EVM"}]}value forhub-v1-last-connected-walletsin localStorage and refreshing the app. Observed that Solana namespace connected successfully and also EVM namespace removed from localstorage.Checklist: