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

Set up Sentry #103

Merged
merged 12 commits into from
Nov 9, 2023
Merged

Set up Sentry #103

merged 12 commits into from
Nov 9, 2023

Conversation

hcho112
Copy link
Collaborator

@hcho112 hcho112 commented Oct 31, 2023

This PR contains initial implementation of Sentry, FE error monitoring tool.

During the implementation, I have created manual throw error on this test scenario to make sure errors are being reported back to sentry.

Things have been tested:

MPC related failure:

  • fail to claim odic
  • get user credential
  • /sign fail

Relayer related failure:

  • signAndSendTransaction

Firebase related failure:

  • create device
  • delete device

Things have not been handled:

Kitwallet related failure where due to indexer falling behind, it sometimes fail to return accountId associated to given public key. (If we search on chain, it exist but kitwallet return empty list instead) This issue has been reported back to @esaminu and he is planning to come up with solution.

Since this PR contains implementation to upload sourcemap to sentry, we need to update github actions to add SENTRY_AUTH_TOKEN to pipeline. This needs to be done before we merge this PR. Please contact me for retrieval of actual token.

Lastly, current setup is currently implemented via sentry account of my own. In the future, we can easily swapped into Pagoda sentry account.

@vercel
Copy link

vercel bot commented Oct 31, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fast-auth-signer ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 3, 2023 2:30am

@esaminu
Copy link
Collaborator

esaminu commented Oct 31, 2023

Currently it seems the lint job is failing. Also can we confirm that sentry and SENTRY_AUTH_TOKEN is not required to build and run the signer app? Some SDK integrators may opt out of Sentry integration

@@ -42,6 +43,7 @@ class FirestoreController {
.then((res) => res.json())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a good idea to capture an exception with the publicKey and email / firebase user id when {indexer}/accounts returns an empty array here and other relevant places we call it wdyt?

src/index.tsx Outdated
Comment on lines 34 to 38
'https://mpc-recovery-leader-mainnet-tmp-cg7nolnlpa-ue.a.run.app',
'https://api.kitwallet.app',
// TESTNET
'https://mpc-recovery-leader-dev-7tk2cmmtcq-ue.a.run.app',
'https://testnet-api.kitwallet.app'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice but should we get these from env?

FIREBASE_MEASUREMENT_ID_TESTNET: 'G-HF2NBGE60S'
})
],
plugins: [new HtmlWebpackPlugin({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be prettified slightly similar to before the change?

Copy link
Collaborator

@esaminu esaminu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hcho112 hcho112 merged commit b684f4b into main Nov 9, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants