Skip to content

Commit

Permalink
Wrap the getFormattedIpfsUrl call in getAssetImageURL in a try catch,…
Browse files Browse the repository at this point in the history
… so the app doesn't crash on certain ipfs urls
  • Loading branch information
danjm committed Oct 17, 2023
1 parent 3bc7780 commit 8fa2db3
Showing 1 changed file with 31 additions and 1 deletion.
32 changes: 31 additions & 1 deletion ui/helpers/utils/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { isObject } from '@metamask/utils';
// eslint-disable-next-line import/no-duplicates
import { isStrictHexString } from '@metamask/utils';
import { CHAIN_IDS, NETWORK_TYPES } from '../../../shared/constants/network';
import { logErrorWithMessage } from '../../../shared/modules/error';
import {
toChecksumHexAddress,
stripHexPrefix,
Expand Down Expand Up @@ -503,7 +504,36 @@ export function getAssetImageURL(image, ipfsGateway) {
}

if (ipfsGateway && image.startsWith('ipfs://')) {
return getFormattedIpfsUrl(ipfsGateway, image, true);
// With v11.1.0, we started seeing errors thrown that included this
// line in the stack trace. The cause is that the `getIpfsCIDv1AndPath`
// method within assets-controllers/src/assetsUtil.ts can throw
// if part of the ipfsUrl, i.e. the `image` variable within this function,
// contains characters not in the Base58 alphabet. Details on that are
// here https://digitalbazaar.github.io/base58-spec/#alphabet. This happens
// with some NFTs, when we attempt to parse part of their IPFS image address
// with the `CID.parse` function (CID is part of the multiform package)
//
// Before v11.1.0 `getFormattedIpfsUrl` was not used in the extension codebase.
// Its use within assets-controllers always ensures that errors are caught
// and ignored. So while we were handling NFTs that can cause this error before,
// we were always catching and ignoring the error. As of PR #20172, we started
// passing all NFTs image URLs to `getAssetImageURL` from nft-items.js, which is
// why we started seeing these errors cause crashes for users in v11.1.0
//
// For the sake of a quick fix, we are wrapping this call in a try-catch, which
// the assets-controllers already do in some form in all cases where this function
// is called. This probably does not affect user experience, as we would not have
// correctly rendered these NFTs before v11.1.0 either (due to the same error
// disuccessed in this code comment).
//
// In the future, we can look into solving the root cause, which might require
// no longer using multiform's CID.parse() method within the assets-controller
try {
return getFormattedIpfsUrl(ipfsGateway, image, true);
} catch (e) {
logErrorWithMessage(e);
return '';
}
}
return image;
}
Expand Down

0 comments on commit 8fa2db3

Please sign in to comment.