-
Notifications
You must be signed in to change notification settings - Fork 17
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
Show a nicer UI when minifront can't connect to the extension, and for other errors #752
Show a nicer UI when minifront can't connect to the extension, and for other errors #752
Conversation
599316e
to
065eaa5
Compare
065eaa5
to
a37a2d1
Compare
export const getChainId = async (): Promise<string | undefined> => { | ||
const { parameters } = await viewClient.appParameters({}); | ||
if (!parameters?.chainId) throw new Error('No chainId in response'); | ||
|
||
return parameters.chainId; | ||
return parameters?.chainId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a little change @turbocrime suggested. We don't need to throw if the chain ID doesn't come back from the server.
const NODE_STATUS_PAGE_URL = | ||
window.location.hostname === 'localhost' ? 'http://localhost:5174' : '/'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this probably needs a .env and webpack define
edit: i guess this is vite
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it seems to be the same as the grpc endpoint uri
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is tricky... we'd want to define the port number in .env, you mean, right? that would mean putting 5174
in node-status
's .env
file, and then importing it from minifront? (Cause I assume we'd also want to use the .env
value in node-status
's package.json
dev
script.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps correct node_status uri but otherwise merge away
<SplashPage title='Penumbra extension unavailble'> | ||
<p className='mb-2'>We can't currently connect to the Penumbra extension.</p> | ||
<p className='mb-2'> | ||
This page may have been left open for too long, causing a timeout. Please reload this page | ||
and see if that fixes the issue. | ||
</p> | ||
<p> | ||
If it doesn't, the RPC node that you're connected to could be down. Check{' '} | ||
<a href={NODE_STATUS_PAGE_URL} className='underline'> | ||
the node's status page | ||
</a>{' '} | ||
and, if it is down, consider switching to a different RPC URL in the Penumbra | ||
extension's settings. | ||
</p> | ||
</SplashPage> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these cases should be readily distinguishable by the error message
Previously, when the RPC node was unavailable, the
<ErrorBoundary />
component would catch the error, stringify it, and render it to the screen.Now, we detect whether the given error is specific to the RPC being unavailable, and we show a message for that case.
Before
After
I also prettified the
<ErrorBoundary />
in both the minifront and node-status apps to show other types of errors inside a<SplashPage />
— e.g.:And lastly, I created a 404 page:
Closes #571
Closes #339