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: expected errors spamming sentry #1514

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open
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
23 changes: 15 additions & 8 deletions packages/extension-core/src/domains/sitesAuthorised/store.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { assert } from "@polkadot/util"
import { convertAddress } from "@talismn/util"

import { SubscribableByIdStorageProvider } from "../../libs/Store"
Expand All @@ -8,6 +7,8 @@ import { AuthorizedSite, AuthorizedSites, ProviderType } from "./types"

const OLD_AUTH_URLS_KEY = "authUrls"

export class SitesAuthorizedError extends Error {}

// exported only for test purposes
export class SitesAuthorizedStore extends SubscribableByIdStorageProvider<
AuthorizedSites,
Expand Down Expand Up @@ -49,15 +50,21 @@ export class SitesAuthorizedStore extends SubscribableByIdStorageProvider<
): Promise<boolean> {
const entry = await this.getSiteFromUrl(url)
const addresses = ethereum ? entry?.ethAddresses : entry?.addresses
assert(addresses, `Site ${url} has not been authorised for Talisman yet`)
assert(addresses.length, `No Talisman wallet accounts are authorised to connect to ${url}`)
if (!addresses) {
const message = `Site ${url} has not been authorised for Talisman yet`
throw new SitesAuthorizedError(message)
}
if (!addresses.length) {
const message = `No Talisman wallet accounts are authorised to connect to ${url}`
throw new SitesAuthorizedError(message)
}

// check the supplied address is authorised to interact with this URL
if (address)
assert(
addresses.includes(convertAddress(address, null)),
`The source ${url} is not allowed to intract with this account.`
)
if (address && !addresses.includes(convertAddress(address, null))) {
const message = `The source ${url} is not allowed to interact with this account.`
throw new SitesAuthorizedError(message)
}

return true
}

Expand Down
10 changes: 6 additions & 4 deletions packages/extension-core/src/domains/talisman/handler/rpc.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import assert from "assert"

import { TabsHandler } from "../../../libs/Handler"
import { chainConnector } from "../../../rpcs/chain-connector"
import { chaindataProvider } from "../../../rpcs/chaindata"
Expand All @@ -12,6 +10,8 @@ import {
UnknownJsonRpcResponse,
} from "../types"

export class ChaindataChainNotFoundError extends Error {}

export default class TalismanRpcHandler extends TabsHandler {
#talismanByGenesisHashSubscriptions = new Map<string, (unsubscribeMethod: string) => void>()

Expand All @@ -21,7 +21,8 @@ export default class TalismanRpcHandler extends TabsHandler {
const { genesisHash, method, params } = request

const chain = await chaindataProvider.chainByGenesisHash(genesisHash)
assert(chain, `Chain with genesisHash '${genesisHash}' not found`)
if (!chain)
throw new ChaindataChainNotFoundError(`Chain with genesisHash '${genesisHash}' not found`)

return await chainConnector.send(chain.id, method, params)
}
Expand All @@ -36,7 +37,8 @@ export default class TalismanRpcHandler extends TabsHandler {
const { genesisHash, subscribeMethod, responseMethod, params, timeout } = request

const chain = await chaindataProvider.chainByGenesisHash(genesisHash)
assert(chain, `Chain with genesisHash '${genesisHash}' not found`)
if (!chain)
throw new ChaindataChainNotFoundError(`Chain with genesisHash '${genesisHash}' not found`)

const unsubscribe = await chainConnector.subscribe(
chain.id,
Expand Down
33 changes: 29 additions & 4 deletions packages/extension-core/src/handlers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import { PORT_EXTENSION } from "extension-shared"
import { log } from "extension-shared"

import { sentry } from "../config/sentry"
import { TalismanNotOnboardedError } from "../domains/app/utils"
import { cleanupEvmErrorMessage, getEvmErrorCause } from "../domains/ethereum/errors"
import { SitesAuthorizedError } from "../domains/sitesAuthorised/store"
import { ChaindataChainNotFoundError } from "../domains/talisman/handler/rpc"
import { MessageTypes, TransportRequestMessage } from "../types"
import { AnyEthRequest } from "../types/domains"
import Extension from "./Extension"
Expand Down Expand Up @@ -124,16 +127,38 @@ const talismanHandler = <TMessageType extends MessageTypes>(
rpcData: evmError.data, // don't use "data" as property name or viem will interpret it differently
isEthProviderRpcError: true,
})
} else {
// log to sentry because we need to know the traceback
sentry.captureException(error)
safePostMessage(port, { id, error: error.message })
return
}

// log to sentry because we need to know the traceback
if (!sentryIgnoreTalismanHandlerError(error)) sentry.captureException(error)
safePostMessage(port, { id, error: error.message })
})
.finally(() => {
// heap cleanup
data.request = null
})
}

/**
* If any of our handlers throw an error, we will (by default) log that error to sentry.
*
* However, there are some errors we expect to see during normal operation.
* These are errors which the frontend knows to look for and handle appropriately.
*
* For example, the error which is thrown when no accounts are authorised for a dapp.
*
* This function is used to distinguish between the errors which we don't expect to see,
* and the errors which we do expect to see - only the latter of which should be logged.
*/
const sentryIgnoreTalismanHandlerError = (error?: unknown): boolean => {
// ignore (don't log to sentry) these errors
if (error instanceof TalismanNotOnboardedError) return true
if (error instanceof SitesAuthorizedError) return true
if (error instanceof ChaindataChainNotFoundError) return true

// log all other errors
return false
}

export default talismanHandler
Loading