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

feat: show more informative rpc error #216

Merged
merged 1 commit into from
Dec 20, 2023
Merged
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
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
"@agoric/cosmic-proto": "^0.3.0",
"@agoric/ertp": "^0.16.2",
"@agoric/inter-protocol": "^0.16.1",
"@agoric/rpc": "^0.7.2",
"@agoric/rpc": "^0.9.0",
"@agoric/smart-wallet": "^0.5.3",
"@agoric/ui-components": "^0.9.0",
"@agoric/web-components": "^0.15.0",
"@agoric/wallet": "^0.18.3",
"@agoric/web-components": "^0.13.2",
"@agoric/zoe": "^0.26.2",
"@endo/eventual-send": "^0.17.6",
"@endo/init": "^0.5.60",
Expand Down
59 changes: 30 additions & 29 deletions src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Suspense, useState, useEffect, lazy } from 'react';
import { useEffect } from 'react';
import { ToastContainer } from 'react-toastify';
import { RouterProvider, createHashRouter } from 'react-router-dom';
import Vaults from 'views/Vaults';
Expand All @@ -10,6 +10,8 @@ import {
currentTimeAtom,
isAppVersionOutdatedAtom,
networkConfigAtom,
rpcNodeAtom,
appStore,
} from 'store/app';
import Root from 'views/Root';
import DisclaimerDialog from 'components/DisclaimerDialog';
Expand All @@ -23,6 +25,8 @@ import { makeAgoricChainStorageWatcher } from '@agoric/rpc';
import 'react-toastify/dist/ReactToastify.css';
import 'styles/globals.css';
import ProvisionSmartWalletDialog from 'components/ProvisionSmartWalletDialog';
import ChainConnectionErrorDialog from 'components/ChainConnectionErrorDialog';
import { useStore } from 'zustand';

const router = createHashRouter([
{
Expand Down Expand Up @@ -78,50 +82,56 @@ const useAppVersionWatcher = () => {
}, [referencedUI, setIsAppVersionOutdated]);
};

const NetworkDropdown = lazy(() => import('./components/NetworkDropdown'));

const App = () => {
const netConfig = useAtomValue(networkConfigAtom);
const [chainStorageWatcher, setChainStorageWatcher] = useAtom(
chainStorageWatcherAtom,
);
const [error, setError] = useState<unknown | null>(null);

const networkDropdown = import.meta.env.VITE_NETWORK_CONFIG_URL ? (
<></>
) : (
<Suspense>
<NetworkDropdown />
</Suspense>
);
const { setChainConnectionError } = useStore(appStore);
const setRpcNode = useSetAtom(rpcNodeAtom);

useEffect(() => {
let isCancelled = false;
if (chainStorageWatcher) return;
const startWatching = async () => {
try {
const { rpc, chainName } = await fetchChainInfo(netConfig.url);
const { rpc, chainName, api } = await fetchChainInfo(netConfig.url);
if (isCancelled) return;
setRpcNode(rpc);
setChainStorageWatcher(
makeAgoricChainStorageWatcher(rpc, chainName, e => {
makeAgoricChainStorageWatcher(api, chainName, e => {
console.error(e);
setError(e);
throw e;
setChainConnectionError(
new Error(
'Received invalid response from API Endpoint: ' + e.message,
),
);
}),
);

watchVbank();
} catch (e) {
if (isCancelled) return;
setError(e);
setChainConnectionError(
new Error(
`Error fetching network config from ${netConfig.url}` +
Copy link
Contributor

Choose a reason for hiding this comment

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

question:
Why is this using

e instanceof Error ? `: ${e.message}` : ''

while just above it's only

e.message

Is it because we know makeAgoricChainStorageWatcher is giving us an instance of Error but we don't know if the catch is?

Copy link
Member

Choose a reason for hiding this comment

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

There's probably value in introducing a helper for use in both places; something like

const makeError = (msg, { cause } = {}) => {
  const longMsg = !cause
    ? msg
    : `${msg}: ${cause instanceof Error ? cause.message : cause}`;
  return Error(longMsg, { cause });
};

(e instanceof Error ? `: ${e.message}` : ''),
),
);
}
};
startWatching();

return () => {
isCancelled = true;
};
}, [setError, netConfig, chainStorageWatcher, setChainStorageWatcher]);
}, [
netConfig,
chainStorageWatcher,
setChainStorageWatcher,
setRpcNode,
setChainConnectionError,
]);

useTimeKeeper();
useAppVersionWatcher();
Expand All @@ -136,21 +146,12 @@ const App = () => {
autoClose={false}
></ToastContainer>
<div className="w-screen max-w-7xl m-auto">
{error ? (
<>
<div className="flex justify-end flex-row space-x-2 items-center mr-6 m-2">
{networkDropdown}
</div>
<div>Error connecting to chain!</div>
<details>{error.toString()}</details>
</>
) : (
<RouterProvider router={router} />
)}
<RouterProvider router={router} />
</div>
<DisclaimerDialog />
<AppVersionDialog />
<ProvisionSmartWalletDialog />
<ChainConnectionErrorDialog />
</div>
);
};
Expand Down
41 changes: 25 additions & 16 deletions src/components/ActionsDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,12 @@ type Props = {
onClose: () => void;
title: string;
body: ReactElement;
primaryAction: DialogAction;
primaryAction?: DialogAction;
secondaryAction?: DialogAction;
primaryActionDisabled?: boolean;
// Whether to initially focus the primary action.
initialFocusPrimary?: boolean;
overflow?: boolean;
};

const ActionsDialog = ({
Expand All @@ -28,6 +29,7 @@ const ActionsDialog = ({
secondaryAction,
primaryActionDisabled = false,
initialFocusPrimary = false,
overflow = false,
}: Props) => {
const primaryButtonRef = useRef(null);

Expand Down Expand Up @@ -61,10 +63,15 @@ const ActionsDialog = ({
leaveFrom="opacity-100 scale-100"
leaveTo="opacity-0 scale-95"
>
<Dialog.Panel className="cursor-default w-full max-w-2xl mx-3 transform overflow-hidden rounded-10 bg-white text-left align-middle shadow-card transition-all">
<Dialog.Panel
className={clsx(
'cursor-default w-full max-w-2xl mx-3 transform rounded-10 bg-white text-left align-middle shadow-card transition-all',
overflow ? 'overflow-visible' : 'overflow-hidden',
)}
>
<Dialog.Title
as="div"
className="font-serif text-2xl text-white font-medium px-8 py-6 bg-interPurple"
className="font-serif text-2xl text-white font-medium px-8 py-6 bg-interPurple rounded-t-10"
>
{title}
</Dialog.Title>
Expand All @@ -80,19 +87,21 @@ const ActionsDialog = ({
{secondaryAction.label}
</button>
)}
<button
ref={primaryButtonRef}
disabled={primaryActionDisabled}
className={clsx(
'transition text-btn-xs flex justify-center rounded border border-transparent text-white px-16 py-3',
primaryActionDisabled
? 'bg-disabled cursor-not-allowed'
: 'bg-interPurple hover:opacity-80 active:opacity-60',
)}
onClick={primaryAction.action}
>
{primaryAction.label}
</button>
{primaryAction && (
<button
ref={primaryButtonRef}
disabled={primaryActionDisabled}
className={clsx(
'transition text-btn-xs flex justify-center rounded border border-transparent text-white px-16 py-3',
primaryActionDisabled
? 'bg-disabled cursor-not-allowed'
: 'bg-interPurple hover:opacity-80 active:opacity-60',
)}
onClick={primaryAction.action}
>
{primaryAction.label}
</button>
)}
</div>
</div>
</Dialog.Panel>
Expand Down
62 changes: 62 additions & 0 deletions src/components/ChainConnectionErrorDialog.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import { useEffect, useState } from 'react';
import { appStore, chainStorageWatcherAtom, rpcNodeAtom } from 'store/app';
import { useAtomValue } from 'jotai';
import ActionsDialog from './ActionsDialog';
import { useStore } from 'zustand';

const ChainConnectionErrorDialog = () => {
const { chainConnectionError } = useStore(appStore);
const chainStorageWatcher = useAtomValue(chainStorageWatcherAtom);
const rpcNode = useAtomValue(rpcNodeAtom);
const [isShowing, setIsShowing] = useState(false);

useEffect(() => {
if (chainConnectionError) {
setIsShowing(true);
}
}, [chainConnectionError]);

const body = (
<div className="mt-2 p-1 max-h-96 overflow-y-auto">
<p className="mb-2">
There was an issue connecting to the chain - likely due to RPC node
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
There was an issue connecting to the chain - likely due to RPC node
There was a problem connecting to the chain - likely due to RPC node

issues. We are working to resolve it with our RPC providers. Please
check back in a couple of hours.
</p>
{chainStorageWatcher && (
<p>
API Endpoint:{' '}
<span className="text-blue-500">{chainStorageWatcher?.apiAddr}</span>
</p>
)}
{rpcNode && (
<p>
RPC Endpoint: <span className="text-blue-500">{rpcNode}</span>
Copy link
Member

Choose a reason for hiding this comment

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

Nit: a blue URL looks like a hyperlink. It might be best to choose a different color and/or replace the <span> with <input readonly>.

</p>
)}
<p>
Error:{' '}
Copy link
Member

@gibson042 gibson042 Dec 20, 2023

Choose a reason for hiding this comment

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

We can avoid repeating "error".

Suggested change
Error:{' '}
Detail:{' '}

Also, is text:{' '} more idiomatic than {'text: '}?

<span className="text-alert">{chainConnectionError?.toString()}</span>
</p>
</div>
);

return (
<ActionsDialog
Copy link
Member

Choose a reason for hiding this comment

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

Not requesting a change, but I guess this is no longer an actions dialog.

title="Chain Connection Error"
body={body}
isOpen={isShowing}
secondaryAction={{
action: () => {
setIsShowing(false);
},
label: 'Dismiss',
}}
onClose={() => {
setIsShowing(false);
}}
/>
);
};

export default ChainConnectionErrorDialog;
17 changes: 8 additions & 9 deletions src/components/ProvisionSmartWalletDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@ import { useEffect, useRef, useState } from 'react';
import { toast } from 'react-toastify';
import {
chainConnectionAtom,
chainStorageWatcherAtom,
provisionToastIdAtom,
rpcNodeAtom,
smartWalletProvisionedAtom,
} from 'store/app';
import { querySwingsetParams } from 'utils/swingsetParams';
import type { ChainStorageWatcher } from 'store/app';
import ActionsDialog from './ActionsDialog';
import { provisionSmartWallet } from 'service/wallet';

Expand All @@ -17,39 +16,39 @@ export const currentTermsIndex = 1;

const feeDenom = 10n ** 6n;

const useSmartWalletFeeQuery = (watcher: ChainStorageWatcher | null) => {
const useSmartWalletFeeQuery = (rpc: string | null) => {
const [smartWalletFee, setFee] = useState<bigint | null>(null);
const [error, setError] = useState<Error | null>(null);

useEffect(() => {
const fetchParams = async () => {
assert(watcher);
assert(rpc);
try {
const params = await querySwingsetParams(watcher.rpcAddr);
const params = await querySwingsetParams(rpc);
console.debug('swingset params', params);
setFee(BigInt(params.params.powerFlagFees[0].fee[0].amount));
} catch (e) {
setError(e as Error);
}
};

if (watcher) {
if (rpc) {
fetchParams();
}
}, [watcher]);
}, [rpc]);

return { smartWalletFee, error };
};

const ProvisionSmartWalletDialog = () => {
const chainConnection = useAtomValue(chainConnectionAtom);
const watcher = useAtomValue(chainStorageWatcherAtom);
const rpc = useAtomValue(rpcNodeAtom);
const [provisionToastId, setProvisionToastId] = useAtom(provisionToastIdAtom);
const smartWalletProvisionRequired = useRef(false);
const [isSmartWalletProvisioned] = useAtom(smartWalletProvisionedAtom);
const [shouldBeOpenIfFeeLoaded, setShouldBeOpenIfFeeLoaded] = useState(false);
const { smartWalletFee, error: smartWalletFeeError } =
useSmartWalletFeeQuery(watcher);
useSmartWalletFeeQuery(rpc);

useEffect(() => {
if (
Expand Down
Loading