-
Notifications
You must be signed in to change notification settings - Fork 198
feat: localised error boundaries #10361
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
Conversation
📝 WalkthroughWalkthroughAdds multiple localized error boundaries and a reusable factory with telemetry; wraps charts, trading flows, asset table, and selected routes with error boundaries; introduces shared fallback UI and translations; extends makeSuspenseful to optionally wrap routes with a page-level error boundary. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Router as Router
participant RouteWrapper as makeSuspenseful(..., {}, true)
participant PageEB as PageErrorBoundary (ErrorPageContent)
participant Susp as React.Suspense
participant Lazy as Lazy Component
User->>Router: navigate
Router->>RouteWrapper: render wrapped route
RouteWrapper->>PageEB: render with page-level error boundary
PageEB->>Susp: render Suspense (Spinner fallback)
Susp->>Lazy: attempt render
alt Loading
Susp-->>User: Spinner
else Error thrown
PageEB->>Sentry: captureException(...)
PageEB->>MixPanel: track Error
PageEB-->>User: ErrorPageContent (with retry)
else Success
Lazy-->>User: page content
end
sequenceDiagram
participant Child as Protected Child
participant REB as react-error-boundary
participant Boundary as createErrorBoundary(...)
participant Sentry as Sentry
participant MP as MixPanel
Child->>REB: throws error
REB->>Boundary: onError(error, info)
Boundary->>Sentry: captureException(error, {tags, contexts})
Boundary->>MP: track('Error', {boundary, message, stack})
REB-->>User: render FallbackComponent (reset available)
User->>REB: click Retry
REB->>Child: resetErrorBoundary -> re-render
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
ac9b48d to
ec2fc90
Compare
8467ec1 to
8e1d1ba
Compare
8e1d1ba to
caa7b65
Compare
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.
Actionable comments posted: 6
🧹 Nitpick comments (23)
src/assets/translations/en/main.json (2)
240-261: Normalize title casing and “Retry” copy for consistency across errorBoundary variantsMinor UX/i18n polish:
- Use consistent Title Case for titles (e.g., “Chart Unavailable”, “Loading Failed”).
- Unify retry labels (“Retry” vs “Try Again”) unless there’s a product reason to differentiate.
Apply this minimal diff to align casing and retry copy:
"errorBoundary": { "component": { "title": "Component Error", "body": "This component encountered an error and couldn't load properly.", "retry": "Retry" }, "chart": { - "title": "Chart unavailable", + "title": "Chart Unavailable", "body": "Unable to load chart data at this time.", "retry": "Retry" }, "trading": { "title": "Trading Error", "body": "The trading interface encountered an error. You can try again or refresh the page.", - "retry": "Try Again" + "retry": "Retry" }, "suspense": { - "title": "Loading failed", + "title": "Loading Failed", "body": "This content failed to load properly.", "retry": "Retry" } },
240-261: Avoid duplicate “Retry” strings; prefer reusing common.retry to ease localization overheadIf feasible, have the fallback components render t('common.retry') instead of dedicated errorBoundary.*.retry keys. This keeps one source of truth and simplifies future localization changes. If the domain-specific retry labels must differ later, the current per-domain keys are fine.
src/components/ErrorBoundary/createErrorBoundary.tsx (3)
26-42: Include richer error context and add missing dependency to useCallbackAdd error.name and error.stack to telemetry, and include errorBoundaryName in the dependency array to satisfy exhaustive-deps and future-proof refactors.
- const handleError = useCallback((error: Error, info: { componentStack: string }) => { + const handleError = useCallback((error: Error, info: { componentStack: string }) => { captureException(error, { tags: { errorBoundary: errorBoundaryName, }, contexts: { react: { componentStack: info.componentStack, }, }, }) getMixPanel()?.track(MixPanelEvent.Error, { - error: error.message, + error: error.message, + name: error.name, + stack: error.stack, errorBoundary: errorBoundaryName, componentStack: info.componentStack, }) - }, []) + }, [errorBoundaryName])
44-47: Preserve FallbackProps precedence to avoid overriding resetErrorBoundary et al.Spread additional props first, then fallbackProps, so core FallbackProps cannot be accidentally overridden by arbitrary extras.
- const FallbackWithProps = useCallback( - (fallbackProps: FallbackProps) => <FallbackComponent {...fallbackProps} {...props} />, - [props], - ) + const FallbackWithProps = useCallback( + (fallbackProps: FallbackProps) => <FallbackComponent {...props} {...fallbackProps} />, + [props], + )
49-55: Expose onReset passthrough and (optionally) set a helpful displayName for DevToolsLet callers provide an onReset handler for boundary resets (useful for clearing localStorage “test*” flags used in manual testing) and pass it to ErrorBoundary. Optionally set a displayName for clearer DevTools traces.
- return function CustomErrorBoundary({ - children, - ...props - }: { - children: ReactNode - [key: string]: unknown - }) { + return function CustomErrorBoundary({ + children, + onReset, + ...props + }: { + children: ReactNode + onReset?: () => void + [key: string]: unknown + }) { const handleError = useCallback((error: Error, info: { componentStack: string }) => { … }, [errorBoundaryName]) const FallbackWithProps = useCallback( (fallbackProps: FallbackProps) => <FallbackComponent {...props} {...fallbackProps} />, [props], ) return ( - <ErrorBoundary FallbackComponent={FallbackWithProps} onError={handleError}> + <ErrorBoundary FallbackComponent={FallbackWithProps} onError={handleError} onReset={onReset}> {children} </ErrorBoundary> ) }If you want a displayName without larger refactors, consider returning a named const instead of an inline function:
const Boundary: React.FC<{ children: ReactNode; onReset?: () => void } & Record<string, unknown>> = ({ children, onReset, ...props }) => { /* ...current body... */ } Boundary.displayName = `${errorBoundaryName}-error-boundary` return Boundarysrc/pages/Markets/components/CardWithSparkline.tsx (1)
83-92: DRY the repeated chart heightBoth the boundary and PriceChart specify '150px'. Define a local constant to avoid divergence.
- <ChartErrorBoundary height='150px'> + <ChartErrorBoundary height={SPARKLINE_HEIGHT}> <PriceChart assetId={assetId} timeframe={HistoryTimeframe.DAY} percentChange={changePercent24Hr} setPercentChange={noop} - chartHeight='150px' + chartHeight={SPARKLINE_HEIGHT} hideAxis={true} /> </ChartErrorBoundary>Add once near the top of the component file:
const SPARKLINE_HEIGHT = '150px'src/components/AssetHeader/AssetChart.tsx (1)
145-155: Avoid duplicating the responsive chart height computationCompute once and pass to both the boundary and PriceChart.
- <ChartErrorBoundary height={isLargerThanMd ? '350px' : '200px'}> + <ChartErrorBoundary height={chartHeight}> <PriceChart assetId={assetId} timeframe={timeframe} percentChange={percentChange} setPercentChange={setPercentChange} setFiatChange={setFiatChange} - chartHeight={isLargerThanMd ? '350px' : '200px'} + chartHeight={chartHeight} hideAxis={true} /> </ChartErrorBoundary>Add above (e.g., after Line 63):
const chartHeight = isLargerThanMd ? '350px' : '200px'src/pages/Trade/StandaloneTrade.tsx (1)
128-132: Enable passthrough for onReset and resetKeys and wire up trade-state resetAfter checking,
createErrorBoundarycurrently only acceptserrorBoundaryNameandFallbackComponent– it does not forwardonResetorresetKeys. To improve recovery UX in the standalone trade flow, let’s:• Update the factory in src/components/ErrorBoundary/createErrorBoundary.tsx to accept and forward
onResetandresetKeys.
• Wire in a reset of the trade input when the boundary resets in src/pages/Trade/StandaloneTrade.tsx.Locations to update:
- Factory signature & render: src/components/ErrorBoundary/createErrorBoundary.tsx
- Boundary usage: src/pages/Trade/StandaloneTrade.tsx (lines 128–132)
Proposed changes:
--- a/src/components/ErrorBoundary/createErrorBoundary.tsx +++ b/src/components/ErrorBoundary/createErrorBoundary.tsx @@ -12,14 +12,20 @@ FallbackComponent: ComponentType<FallbackProps & Record<string, unknown>> } -export function createErrorBoundary({ - errorBoundaryName, - FallbackComponent, -}: CreateErrorBoundaryOptions) { +export function createErrorBoundary({ + errorBoundaryName, + FallbackComponent, +}: CreateErrorBoundaryOptions) { return function CustomErrorBoundary({ - children, - ...props + children, + onReset, + resetKeys, + ...props }: { children: ReactNode + onReset?: () => void + resetKeys?: unknown[] [key: string]: unknown }) { const handleError = useCallback((error: Error, info: { componentStack: string }) => { @@ -29,7 +35,9 @@ const FallbackWithProps = useCallback( (fallbackProps: FallbackProps) => <FallbackComponent {...fallbackProps} {...props} />, [props], - ) + ) return ( - <ErrorBoundary FallbackComponent={FallbackWithProps} onError={handleError}> + <ErrorBoundary + FallbackComponent={FallbackWithProps} + onError={handleError} + onReset={onReset} + resetKeys={resetKeys} + > {children} </ErrorBoundary> )--- a/src/pages/Trade/StandaloneTrade.tsx +++ b/src/pages/Trade/StandaloneTrade.tsx @@ 128,7 +128,13 @@ () => ( - <TradingErrorBoundary> + <TradingErrorBoundary + onReset={() => { + dispatch(tradeInput.actions.clear()) + }} + resetKeys={[props.defaultBuyAssetId, props.defaultSellAssetId]} + > <StandaloneMultiHopTrade {...props} onChangeTab={handleChangeTab} isStandalone /> - </TradingErrorBoundary> + </TradingErrorBoundary> ), [dispatch, handleChangeTab, props.defaultBuyAssetId, props.defaultSellAssetId, props], )With these changes, retrying the error boundary will also clear the trade inputs and remount the flow whenever the default assets change, improving recovery flow for users.
src/components/ErrorBoundary/TradingErrorBoundary.tsx (2)
25-28: Boundary creation looks good; consider console visibility in dev and reset plumbing
createErrorBoundarylogs to Sentry/Mixpanel, which satisfies telemetry. The linked issue also asks that errors are visible in the console. Consider adding a dev-onlyconsole.error(error, info.componentStack)in the factory’sonError. Also forwardonReset/resetKeys(see earlier suggestion) so pages can clear state or auto-reset on param changes.
11-23: PreferFallbackPropsfromreact-error-boundary& (optional) surface an error toast– Import
FallbackPropsand tighten the component’s signature to match the upstream API.
– Optional: import and invoke youruseErrorToasthook on first mount so users see an error notification (guarded by a ref to avoid duplicates).Example diff:
import { useTranslate } from 'react-polyglot' +import type { FallbackProps } from 'react-error-boundary' +import { useEffect, useRef } from 'react' +import { useErrorToast } from 'src/hooks/useErrorToast/useErrorToast' -// before: custom prop type -const TradingErrorFallback: React.FC<TradingErrorFallbackProps> = ({ resetErrorBoundary }) => { +const TradingErrorFallback: React.FC<FallbackProps> = ({ error, resetErrorBoundary }) => { const translate = useTranslate() + const toast = useErrorToast() + const hasToasted = useRef(false) + + useEffect(() => { + if (!hasToasted.current) { + toast({ + title: translate('errorBoundary.trading.toastTitle'), + description: error.message, + }) + hasToasted.current = true + } + }, [error, toast, translate]) return ( <ErrorFallback title={translate('errorBoundary.trading.title')} body={translate('errorBoundary.trading.body')} retryLabel={translate('errorBoundary.trading.retry')} onRetry={resetErrorBoundary} size='lg' /> ) }src/components/ErrorPage/ErrorPageContent.tsx (1)
25-40: Avoid duplicating the error UI — reuse the shared ErrorFallback (lg) variantYou already introduced a reusable ErrorFallback. Reusing it here keeps visuals consistent with other boundaries and simplifies future changes.
Apply this diff:
-import { Button, Center, Heading, Stack } from '@chakra-ui/react' -import { useCallback } from 'react' -import { FaSadTear } from 'react-icons/fa' +import { useCallback } from 'react' import { useTranslate } from 'react-polyglot' -import { IconCircle } from '@/components/IconCircle' -import { RawText } from '@/components/Text' +import { ErrorFallback } from '@/components/ErrorBoundary/ErrorFallback' @@ export const ErrorPageContent: React.FC<ErrorPageContentProps> = ({ resetErrorBoundary }) => { const translate = useTranslate() const handleReset = useCallback(() => { if (resetErrorBoundary) { resetErrorBoundary() } else { // Default behavior: reload the page window.location.reload() } }, [resetErrorBoundary]) - return ( - <Center width='full' height='100%'> - <Stack justifyContent='center' alignItems='center'> - <IconCircle fontSize='6xl' boxSize='16' bg='blue.500' color='white'> - <FaSadTear /> - </IconCircle> - <Heading lineHeight='shorter' fontSize='6xl'> - {translate('errorPage.title')} - </Heading> - <RawText fontSize='xl'>{translate('errorPage.body')}</RawText> - <Button colorScheme='blue' onClick={handleReset}> - {translate('errorPage.cta')} - </Button> - </Stack> - </Center> - ) + return ( + <ErrorFallback + size='lg' + title={translate('errorPage.title')} + body={translate('errorPage.body')} + retryLabel={translate('errorPage.cta')} + onRetry={handleReset} + /> + ) }src/components/ErrorBoundary/ComponentErrorBoundary.tsx (1)
11-23: Optional: Surface a toast with localized error summary when the boundary tripsGuidelines recommend using useErrorToast for boundary errors. Consider showing a one-shot toast on mount for additional context (without leaking PII).
Illustrative pattern:
+import { useEffect } from 'react' +import { useErrorToast } from '@/components/Toast/useErrorToast' @@ -const ComponentErrorFallback: React.FC<ComponentErrorFallbackProps> = ({ resetErrorBoundary }) => { +const ComponentErrorFallback: React.FC<ComponentErrorFallbackProps> = ({ error, resetErrorBoundary }) => { const translate = useTranslate() + const errorToast = useErrorToast() + + useEffect(() => { + // Generic, localized toast without sensitive details + errorToast({ + title: translate('errorBoundary.component.title'), + description: translate('errorBoundary.component.body'), + // optionally map to severity, duration, etc. + }) + }, [errorToast, translate])src/utils/makeSuspenseful.tsx (2)
32-35: Rephrase lint comment and fix typoThe comment is informal and contains a typo (“dependancy”). Keep it professional and explanatory.
Apply this diff:
-// eslint you're drunk, this is a module-scope element with a module-scope const dependancy -// eslint-disable-next-line react-memo/require-usememo +// Note: defaultSuspenseFallback is a module-scope element derived from a module-scope constant. +// Disabling react-memo/require-usememo is intentional here to avoid unnecessary memoization. +// eslint-disable-next-line react-memo/require-usememo
47-55: Optional: Preserve component names in React DevToolsSetting a displayName helps debugging and boundary error reports.
Apply this diff:
return (props: ComponentProps<T>) => { @@ - return withErrorBoundary ? ( + const element = withErrorBoundary ? ( <PageErrorBoundary> <Suspense fallback={suspenseSpinner}> <Component {...props} /> </Suspense> </PageErrorBoundary> ) : ( <Suspense fallback={suspenseSpinner}> <Component {...props} /> </Suspense> ) + // Name the wrapper for DevTools and error reports + ;(element as any).type.displayName = + (Component as any).displayName || (Component as any).name || 'Anonymous' + return elementIf you adopt the earlier generic refactor, set the displayName on the returned component instead:
- return (props: TProps) => { + const Wrapped: FC<TProps> = props => { @@ - } + } + Wrapped.displayName = `Suspenseful(${(Component as any).displayName ?? Component.name ?? 'Component'})` + return Wrappedsrc/pages/ErrorPage/ErrorPage.tsx (1)
7-12: Pass resetErrorBoundary through to content so CTA can “retry” without a full reloadErrorPage is typed as React.FC but doesn’t forward resetErrorBoundary, causing the CTA to fallback to window reload. Forward it so users can retry in-place when this page is used as a boundary fallback.
Apply this diff:
-export const ErrorPage: React.FC<FallbackProps> = () => { +export const ErrorPage: React.FC<FallbackProps> = ({ resetErrorBoundary }) => { return ( <Layout display='flex'> <Main height='100%' display='flex' width='full'> - <ErrorPageContent /> + <ErrorPageContent resetErrorBoundary={resetErrorBoundary} /> </Main> </Layout> ) }src/components/ErrorBoundary/ErrorFallback.tsx (2)
19-50: Type the sizeConfig map for stronger guaranteesMake sizeConfig a typed record to ensure all sizes are covered and property types are consistent.
Apply this diff:
-const sizeConfig = { +const sizeConfig: Record< + ErrorFallbackSize, + { + containerPadding: number + stackSpacing: number + iconSize: string + iconFontSize: string + headingSize: string + textSize: string + buttonSize: string + maxWidth?: string + } +> = { sm: { @@ }, }
63-93: Minor a11y improvements: announce errors and hide decorative icon from AT
- Mark the container as an alert so SR users are notified when the fallback appears.
- Hide the default decorative icon from screen readers.
Apply this diff:
- return ( - <Center p={size === 'lg' ? 6 : 4}> + return ( + <Center p={size === 'lg' ? 6 : 4}> <Box p={config.containerPadding} borderRadius='xl' bg='background.surface.raised.base' border='1px' borderColor='border.base' maxW={config.maxWidth} + role='alert' + aria-live='polite' > @@ - > - {icon || <FaSadTear />} + > + {icon || <FaSadTear aria-hidden='true' focusable='false' />} </IconCircle>src/components/ErrorBoundary/ChartErrorBoundary.tsx (2)
50-58: Avoid remounts when only the height changesRecreating the boundary component type when the
heightprop changes will remount the boundary and its children, resetting internal state (including chart caches) during responsive breakpoints. Prefer keeping the boundary component identity stable and providingheightto the fallback without regenerating the boundary.Two options:
- Option A (recommended): Extend
createErrorBoundaryto accept afallbackRenderorfallbackPropsso you can passheightat render time while keeping the boundary stable.- Option B: Keep this as-is for now and follow up if you see state resets in responsive scenarios.
If you go with Option A, the call site here could look like this (assuming the factory supports
fallbackRender):- const BaseErrorBoundary = useMemo( - () => - createErrorBoundary({ - errorBoundaryName: 'chart', - FallbackComponent: props => <ChartErrorFallback {...props} height={height} />, - }), - [height], - ) + const BaseErrorBoundary = useMemo( + () => + createErrorBoundary({ + errorBoundaryName: 'chart', + fallbackRender: props => <ChartErrorFallback {...props} height={height} />, + }), + [], // factory result is stable; height flows via fallbackRender + )Would you like a follow-up patch to extend
createErrorBoundarywithfallbackRendersupport while preserving typings?
25-41: Consider reusing the shared ErrorFallback to reduce duplicationYou already have a generic
ErrorFallbackcomponent. Reusing it here would keep visuals consistent and reduce maintenance. You can still preserve the height wrapper.Proposed change (requires adding
import { ErrorFallback } from './ErrorFallback'):- return ( - <Center h={height} borderRadius='lg' bg='background.surface.raised.base' p={4}> - <Stack spacing={3} align='center' textAlign='center'> - <IconCircle fontSize='2xl' boxSize='10' bg='border.base' color='text.subtle'> - {chartIcon} - </IconCircle> - <RawText fontSize='md' fontWeight='semibold' color='text.base'> - {translate('errorBoundary.chart.title')} - </RawText> - <RawText fontSize='sm' color='text.subtle'> - {translate('errorBoundary.chart.body')} - </RawText> - <Button size='sm' colorScheme='blue' onClick={resetErrorBoundary} px={6}> - {translate('errorBoundary.chart.retry')} - </Button> - </Stack> - </Center> - ) + return ( + <Center h={height}> + <ErrorFallback + icon={chartIcon} + title={translate('errorBoundary.chart.title')} + body={translate('errorBoundary.chart.body')} + retryLabel={translate('errorBoundary.chart.retry')} + onRetry={resetErrorBoundary} + size='sm' + /> + </Center> + )Note: This nests centers, but the outer one ensures height; if you prefer, we can extend
ErrorFallbackto accept an optionalcontainerHeightprop and remove the outerCenter.src/components/ErrorBoundary/SuspenseErrorBoundary.tsx (3)
51-67: Enrich telemetry payload for faster triageYou're tracking
error.message, boundary tag, and component stack. Consider addingerror.nameand a truncatederror.stack(or hashed fingerprint) to improve deduplication and triage in Mixpanel, mirroring what Sentry already captures.- getMixPanel()?.track(MixPanelEvent.Error, { - error: error.message, - errorBoundary: 'suspense', - componentStack: info.componentStack, - }) + getMixPanel()?.track(MixPanelEvent.Error, { + error: error.message, + errorName: error.name, + // Trim stack to first 1-2 lines to reduce PII risk and payload size + errorStack: (error.stack || '').split('\n').slice(0, 2).join('\n'), + errorBoundary: 'suspense', + componentStack: info.componentStack, + })
71-75: Unify with the error-boundary factory for consistencyElsewhere we centralize error reporting via
createErrorBoundary. Consider using the factory here too so all boundaries share the same logging behavior, redaction, and future tweaks.Example refactor:
- return ( - <ErrorBoundary FallbackComponent={FallbackComponent} onError={handleError}> - <Suspense fallback={suspenseFallback}>{children}</Suspense> - </ErrorBoundary> - ) + const BaseErrorBoundary = useMemo( + () => + createErrorBoundary({ + errorBoundaryName: 'suspense', + FallbackComponent, + }), + [FallbackComponent], + ) + return ( + <BaseErrorBoundary> + <Suspense fallback={suspenseFallback}>{children}</Suspense> + </BaseErrorBoundary> + )This assumes
createErrorBoundaryencapsulates Sentry and Mixpanel logging for the 'suspense' boundary, in which casehandleErrorand its imports can be removed.If helpful, I can open a small follow-up PR to perform this unification and remove the duplicate logging logic here.
21-30: Toast feedback on error (optional)Acceptance criteria call for errors not being swallowed and visible in console, which you meet. Optionally, also show a user toast when the error fallback renders (via the fallback component) to proactively communicate issues. Use
useErrorToastwith translated messages undererrorBoundary.suspense.*.src/Routes/RoutesCommon.tsx (1)
40-48: Prefer explicit options over{}for readabilityPassing
{}as the second arg is a little opaque in call sites. IfmakeSuspensefulallows an optional second parameter, consider either:
- Overload/intake where the options param is optional so you can call
makeSuspenseful(lazyComp, undefined, true), or- Define a named constant like
DEFAULT_SUSPENSE_OPTIONSto make intent clearer.Low priority; leaving as-is is fine if the signature requires an object.
Also applies to: 50-58, 60-68, 70-78, 80-88, 90-98, 100-108, 110-118, 120-128
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
src/Routes/RoutesCommon.tsx(9 hunks)src/assets/translations/en/main.json(1 hunks)src/components/AssetHeader/AssetChart.tsx(2 hunks)src/components/ErrorBoundary/ChartErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/ComponentErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/ErrorFallback.tsx(1 hunks)src/components/ErrorBoundary/SuspenseErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/TradingErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/createErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/index.ts(1 hunks)src/components/ErrorPage/ErrorPageContent.tsx(1 hunks)src/pages/Assets/Assets.tsx(2 hunks)src/pages/Dashboard/components/DashboardChart.tsx(2 hunks)src/pages/ErrorPage/ErrorPage.tsx(1 hunks)src/pages/Markets/components/CardWithSparkline.tsx(2 hunks)src/pages/Trade/StandaloneTrade.tsx(2 hunks)src/pages/Trade/tabs/TradeTab.tsx(2 hunks)src/utils/makeSuspenseful.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/pages/Markets/components/CardWithSparkline.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/AssetHeader/AssetChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsxsrc/Routes/RoutesCommon.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/assets/translations/en/main.jsonsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/pages/Markets/components/CardWithSparkline.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/AssetHeader/AssetChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsxsrc/Routes/RoutesCommon.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/pages/Markets/components/CardWithSparkline.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/AssetHeader/AssetChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsxsrc/Routes/RoutesCommon.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/pages/Markets/components/CardWithSparkline.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/AssetHeader/AssetChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsxsrc/Routes/RoutesCommon.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/pages/Markets/components/CardWithSparkline.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/AssetHeader/AssetChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsxsrc/Routes/RoutesCommon.tsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: gomesalexandre
PR: shapeshift/web#10220
File: src/pages/Explore/components/Tags.tsx:18-118
Timestamp: 2025-08-07T16:09:41.502Z
Learning: gomesalexandre indicated that error boundaries are not necessary for DOM manipulation in components like Tags. He prefers focusing error boundaries around suspense boundaries and implementing them more strategically as local error boundaries rather than wrapping every component.
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS wrap components in error boundaries
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS provide user-friendly fallback components in error boundaries
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : Missing error boundaries in React components is an anti-pattern
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS wrap components in error boundaries for production
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS log errors for debugging in error boundaries
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : Missing error boundaries in React components is an anti-pattern
Applied to files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS wrap components in error boundaries
Applied to files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/pages/Dashboard/components/DashboardChart.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsx
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS wrap components in error boundaries for production
Applied to files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS provide user-friendly fallback components in error boundaries
Applied to files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorPage/ErrorPageContent.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/pages/ErrorPage/ErrorPage.tsxsrc/utils/makeSuspenseful.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS log errors for debugging in error boundaries
Applied to files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsxsrc/pages/Assets/Assets.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/ChartErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/utils/makeSuspenseful.tsx
📚 Learning: 2025-08-07T16:09:41.502Z
Learnt from: gomesalexandre
PR: shapeshift/web#10220
File: src/pages/Explore/components/Tags.tsx:18-118
Timestamp: 2025-08-07T16:09:41.502Z
Learning: gomesalexandre indicated that error boundaries are not necessary for DOM manipulation in components like Tags. He prefers focusing error boundaries around suspense boundaries and implementing them more strategically as local error boundaries rather than wrapping every component.
Applied to files:
src/components/ErrorBoundary/index.tssrc/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsxsrc/utils/makeSuspenseful.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : ALWAYS provide fallback error handling
Applied to files:
src/components/ErrorBoundary/ComponentErrorBoundary.tsxsrc/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/components/ErrorBoundary/ErrorFallback.tsxsrc/components/ErrorBoundary/SuspenseErrorBoundary.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/swapper/**/*.{ts,tsx} : ALWAYS use TradeQuoteError enum for error codes in swapper errors
Applied to files:
src/components/ErrorBoundary/TradingErrorBoundary.tsxsrc/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS provide translated error messages in error toasts
Applied to files:
src/components/ErrorBoundary/TradingErrorBoundary.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
PR: shapeshift/web#10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/pages/Trade/StandaloneTrade.tsxsrc/pages/Trade/tabs/TradeTab.tsx
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.
Applied to files:
src/pages/Trade/tabs/TradeTab.tsx
📚 Learning: 2025-08-08T11:41:36.971Z
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.
Applied to files:
src/pages/Trade/tabs/TradeTab.tsx
📚 Learning: 2025-07-30T20:57:48.176Z
Learnt from: NeOMakinG
PR: shapeshift/web#10148
File: src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx:88-91
Timestamp: 2025-07-30T20:57:48.176Z
Learning: In src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx, NeOMakinG prefers keeping the hardcoded overscan value (40) over dynamic calculation based on viewport height, prioritizing code simplicity over precision when the current approach works effectively.
Applied to files:
src/pages/Assets/Assets.tsx
📚 Learning: 2025-08-05T22:41:35.473Z
Learnt from: premiumjibles
PR: shapeshift/web#10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
Applied to files:
src/pages/Markets/components/CardWithSparkline.tsxsrc/components/AssetHeader/AssetChart.tsx
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/pages/Explore/ExploreCategory.tsx:231-238
Timestamp: 2025-08-08T14:59:40.422Z
Learning: In src/pages/Explore/ExploreCategory.tsx, for the PageHeader filter trigger, NeOMakinG considers changing a clickable Chakra Icon to IconButton too nitpicky for this PR and prefers to keep the current Icon-based trigger; such minor a11y/UI nitpicks should be deferred to a follow-up if needed.
Applied to files:
src/pages/ErrorPage/ErrorPage.tsx
🧬 Code graph analysis (13)
src/components/ErrorBoundary/ComponentErrorBoundary.tsx (2)
src/components/ErrorBoundary/ErrorFallback.tsx (1)
ErrorFallback(52-94)src/components/ErrorBoundary/createErrorBoundary.tsx (1)
createErrorBoundary(15-55)
src/components/ErrorBoundary/TradingErrorBoundary.tsx (2)
src/components/ErrorBoundary/ErrorFallback.tsx (1)
ErrorFallback(52-94)src/components/ErrorBoundary/createErrorBoundary.tsx (1)
createErrorBoundary(15-55)
src/pages/Trade/StandaloneTrade.tsx (2)
src/components/ErrorBoundary/TradingErrorBoundary.tsx (1)
TradingErrorBoundary(25-28)src/components/MultiHopTrade/StandaloneMultiHopTrade.tsx (1)
StandaloneMultiHopTrade(36-140)
src/pages/Trade/tabs/TradeTab.tsx (2)
src/components/ErrorBoundary/TradingErrorBoundary.tsx (1)
TradingErrorBoundary(25-28)src/components/MultiHopTrade/MultiHopTrade.tsx (1)
MultiHopTrade(39-112)
src/pages/Assets/Assets.tsx (2)
src/components/ErrorBoundary/ComponentErrorBoundary.tsx (1)
ComponentErrorBoundary(25-28)src/components/MarketTableVirtualized/MarketsTableVirtualized.tsx (1)
MarketsTableVirtualized(80-271)
src/components/ErrorBoundary/ErrorFallback.tsx (1)
src/components/Text/Text.tsx (1)
RawText(15-17)
src/pages/Markets/components/CardWithSparkline.tsx (2)
src/components/ErrorBoundary/index.ts (1)
ChartErrorBoundary(2-2)src/components/PriceChart/PriceChart.tsx (1)
PriceChart(31-109)
src/components/ErrorBoundary/createErrorBoundary.tsx (1)
src/lib/mixpanel/mixPanelSingleton.ts (1)
getMixPanel(9-41)
src/components/ErrorPage/ErrorPageContent.tsx (1)
src/components/Text/Text.tsx (1)
RawText(15-17)
src/pages/Dashboard/components/DashboardChart.tsx (2)
src/components/ErrorBoundary/index.ts (1)
ChartErrorBoundary(2-2)src/components/BalanceChart/BalanceChart.tsx (1)
BalanceChart(26-56)
src/components/ErrorBoundary/SuspenseErrorBoundary.tsx (2)
src/components/ErrorBoundary/ErrorFallback.tsx (1)
ErrorFallback(52-94)src/lib/mixpanel/mixPanelSingleton.ts (1)
getMixPanel(9-41)
src/pages/ErrorPage/ErrorPage.tsx (3)
src/components/Layout/Layout.tsx (1)
Layout(15-37)src/components/Layout/Main.tsx (1)
Main(16-27)src/components/ErrorPage/ErrorPageContent.tsx (1)
ErrorPageContent(13-41)
src/utils/makeSuspenseful.tsx (2)
src/components/ErrorBoundary/createErrorBoundary.tsx (1)
createErrorBoundary(15-55)src/components/ErrorPage/ErrorPageContent.tsx (1)
ErrorPageContent(13-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Call / Static
🔇 Additional comments (15)
src/assets/translations/en/main.json (1)
240-261: New errorBoundary translations look good and align with localized fallbacksThe added keys provide clear, user-friendly copy for component-, chart-, trading-, and suspense-specific boundaries and map well to the new boundary components. No conflicts with existing errorPage keys.
src/components/AssetHeader/AssetChart.tsx (1)
145-155: LGTM; boundary placement is correct and scoped to the chartWrapping just the chart avoids taking down the entire asset header if the chart explodes. Nice.
src/pages/Assets/Assets.tsx (2)
78-84: LGTM; localized boundary around the virtualized markets table is appropriateGood scope: a row rendering bug won’t nuke the entire Assets page. Consider wiring an optional onReset to clear any table-local caches if needed in the future, but not required for this PR.
10-10: Exports Confirmed:ComponentErrorBoundaryis correctly exported
The barrel file atsrc/components/ErrorBoundary/index.tsincludes a named export forComponentErrorBoundary, so importing it via@/components/ErrorBoundaryis valid.src/pages/Trade/StandaloneTrade.tsx (1)
6-6: LGTM: Localized trading boundary import aligns with PR goalsUsing the barrel export keeps imports consistent and matches the new error-boundary system.
src/pages/Trade/tabs/TradeTab.tsx (1)
7-7: LGTM: Boundary import via barrel is consistentMatches the new error-boundary module structure.
src/pages/Dashboard/components/DashboardChart.tsx (2)
22-22: LGTM: ChartErrorBoundary importConsistent with the new error boundary exports.
133-142: Consider adding resetKeys to ChartErrorBoundaryTranslation keys for the chart error boundary are present in
src/assets/translations/en/main.json(around lines 246–249), so you can safely addresetKeysonce factory support is in place:- <ChartErrorBoundary height='400px'> + <ChartErrorBoundary height='400px' resetKeys={[timeframe, isRainbowChart]}> <Suspense fallback={balanceChartFallback}> <BalanceChart timeframe={timeframe} percentChange={percentChange} setPercentChange={setPercentChange} isRainbowChart={isRainbowChart} /> </Suspense> </ChartErrorBoundary>This optional change will reset the boundary whenever the timeframe or chart mode changes, preventing it from getting “stuck.”
src/components/ErrorBoundary/index.ts (1)
1-4: LGTM: Barrel re-exports are clear and minimalKeeps imports uniform and prevents deep path imports.
src/components/ErrorBoundary/ComponentErrorBoundary.tsx (1)
11-23: LGTM: Fallback wiring and i18n are correctFallback receives resetErrorBoundary and delegates to ErrorFallback; createErrorBoundary will log to Sentry/Mixpanel. This aligns with the localized boundary goals.
src/components/ErrorBoundary/ChartErrorBoundary.tsx (2)
19-43: Solid, localized chart fallback UIClear, localized copy, sensible defaults, and a focused retry affordance. The domain-specific icon and height control are nice touches. No blockers.
31-39: Translation Keys Verified Across LocalesAll
errorBoundary.chart.title,errorBoundary.chart.body, anderrorBoundary.chart.retrykeys exist insrc/assets/translations/en/main.jsonand have been confirmed in all other locale files.src/components/ErrorBoundary/SuspenseErrorBoundary.tsx (1)
32-36: Good loading fallback with height controlSimple, accessible loading state that respects container height. This keeps route skeletons consistent while data loads. Looks good.
src/Routes/RoutesCommon.tsx (2)
40-48: Enabling page-level error boundaries on lazy routes is the right moveSwitching these
makeSuspenseful(...)calls to passwithErrorBoundary: trueshould localize crashes to the route and surface a consistent error UI instead of blank states. This aligns with the PR goals and reduces blast radius.Also applies to: 50-58, 60-68, 70-78, 80-88, 90-98, 100-108, 110-118, 120-128
138-227: PII routing note: verify instrumentation remains compliantGiven the critical note about routes and Mixpanel, double-check that the page-level error boundary wrapper doesn't alter the parsed route strings used by Mixpanel helpers. The main component identity remains the same, so it should be fine, but please re-run the existing tests mentioned in the comment after enabling boundaries on these routes.
NeOMakinG
left a comment
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.
https://jam.dev/c/5689db53-643d-4fbc-bed4-f76cf74cbbf5
I couldn't reproduce a few error boundaries, might be dude to my local state anyway so not blocking for this after reading the code, it's quite straightforward
One thinking thought, why not adding a system to make error boundaries failing somehow, maybe by using a feature flag? So we can test this easily in the future
ff9adc9 to
ee0a694
Compare
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.
Actionable comments posted: 0
🧹 Nitpick comments (7)
src/components/ErrorBoundary/index.ts (1)
1-4: Optional: Re-export the factory for a single import surface.If we intend consumers to import all boundary-related utilities from one place, consider re-exporting
createErrorBoundaryhere as well for consistency.export { ComponentErrorBoundary } from './ComponentErrorBoundary' export { ChartErrorBoundary } from './ChartErrorBoundary' export { TradingErrorBoundary } from './TradingErrorBoundary' export { SuspenseErrorBoundary } from './SuspenseErrorBoundary' +export { createErrorBoundary } from './createErrorBoundary'src/components/ErrorBoundary/createErrorBoundary.tsx (6)
10-13: Make the factory generic over fallback props for strong typing (avoid index signatures).Current typing forces a
Record<string, unknown>on the fallback component, which is overly permissive and can conflict with specific fallback props (e.g.,heightfor chart fallbacks). Use generics so callers get proper type safety on boundary props and fallback component props.-import type { ComponentType, ReactNode } from 'react' +import type { ComponentType, PropsWithChildren } from 'react' import { useCallback } from 'react' @@ -type CreateErrorBoundaryOptions = { - errorBoundaryName: string - FallbackComponent: ComponentType<FallbackProps & Record<string, unknown>> -} +type CreateErrorBoundaryOptions<P extends object = Record<string, never>> = { + errorBoundaryName: string + FallbackComponent: ComponentType<FallbackProps & P> +} @@ -export function createErrorBoundary({ - errorBoundaryName, - FallbackComponent, -}: CreateErrorBoundaryOptions) { - return function CustomErrorBoundary({ - children, - ...props - }: { - children: ReactNode - [key: string]: unknown - }) { +export function createErrorBoundary<P extends object = Record<string, never>>({ + errorBoundaryName, + FallbackComponent, +}: CreateErrorBoundaryOptions<P>) { + return function CustomErrorBoundary({ children, ...props }: PropsWithChildren<P>) {Also applies to: 15-25, 2-3
44-47: PreferfallbackRenderover wrappingFallbackComponentto merge props.Using
fallbackRenderis the intended API for injecting extra props. This removes an extra component layer and avoids identity changes from memoized wrapper components.- const FallbackWithProps = useCallback( - (fallbackProps: FallbackProps) => <FallbackComponent {...fallbackProps} {...props} />, - [props], - ) - - return ( - <ErrorBoundary FallbackComponent={FallbackWithProps} onError={handleError}> + return ( + <ErrorBoundary + fallbackRender={(fallbackProps: FallbackProps) => ( + <FallbackComponent {...fallbackProps} {...props} /> + )} + onError={handleError} + > {children} </ErrorBoundary> )Also applies to: 49-53
26-43: IncludeerrorBoundaryNameinuseCallbackdeps.Keeps us aligned with exhaustive-deps best practices. While stable here, it documents intent and avoids future surprises.
- const handleError = useCallback((error: Error, info: { componentStack: string }) => { + const handleError = useCallback((error: Error, info: { componentStack: string }) => { captureException(error, { @@ getMixPanel()?.track(MixPanelEvent.Error, { error: error.message, errorBoundary: errorBoundaryName, componentStack: info.componentStack, }) - }, []) + }, [errorBoundaryName])
2-3: Type the error info parameter with React’sErrorInfofor clarity.This communicates intent and aligns with React’s built-in types.
-import type { ComponentType, ReactNode } from 'react' +import type { ComponentType, PropsWithChildren, ErrorInfo } from 'react' @@ - const handleError = useCallback((error: Error, info: { componentStack: string }) => { + const handleError = useCallback((error: Error, info: ErrorInfo) => {Also applies to: 26-36
19-25: Optional: SetdisplayNameon the generated component for better DevTools visibility.Helps quickly identify which boundary fired in React DevTools and error overlays.
- return function CustomErrorBoundary({ children, ...props }: PropsWithChildren<P>) { + const CustomErrorBoundary = ({ children, ...props }: PropsWithChildren<P>) => { @@ - ) - } + ) + } + CustomErrorBoundary.displayName = `${errorBoundaryName[0]?.toUpperCase()}${errorBoundaryName.slice(1)}ErrorBoundary` + return CustomErrorBoundaryAlso applies to: 49-53
37-41: Telemetry enrichment (optional).Consider adding
nameandstack(redacted if needed) to the Mixpanel payload to aid triage; Sentry already captures these.- getMixPanel()?.track(MixPanelEvent.Error, { - error: error.message, - errorBoundary: errorBoundaryName, - componentStack: info.componentStack, - }) + getMixPanel()?.track(MixPanelEvent.Error, { + error: error.message, + name: error.name, + errorBoundary: errorBoundaryName, + componentStack: info.componentStack, + })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
src/Routes/RoutesCommon.tsx(9 hunks)src/assets/translations/en/main.json(1 hunks)src/components/AssetHeader/AssetChart.tsx(2 hunks)src/components/ErrorBoundary/ChartErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/ComponentErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/ErrorFallback.tsx(1 hunks)src/components/ErrorBoundary/SuspenseErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/TradingErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/createErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/index.ts(1 hunks)src/components/ErrorPage/ErrorPageContent.tsx(1 hunks)src/pages/Assets/Assets.tsx(2 hunks)src/pages/Dashboard/components/DashboardChart.tsx(2 hunks)src/pages/ErrorPage/ErrorPage.tsx(1 hunks)src/pages/Markets/components/CardWithSparkline.tsx(2 hunks)src/pages/Trade/StandaloneTrade.tsx(2 hunks)src/pages/Trade/tabs/TradeTab.tsx(2 hunks)src/utils/makeSuspenseful.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- src/pages/Trade/StandaloneTrade.tsx
- src/pages/ErrorPage/ErrorPage.tsx
- src/Routes/RoutesCommon.tsx
- src/pages/Trade/tabs/TradeTab.tsx
- src/components/ErrorBoundary/ComponentErrorBoundary.tsx
- src/pages/Assets/Assets.tsx
- src/pages/Markets/components/CardWithSparkline.tsx
- src/assets/translations/en/main.json
- src/components/ErrorBoundary/SuspenseErrorBoundary.tsx
- src/components/AssetHeader/AssetChart.tsx
- src/components/ErrorBoundary/TradingErrorBoundary.tsx
- src/components/ErrorBoundary/ChartErrorBoundary.tsx
- src/pages/Dashboard/components/DashboardChart.tsx
- src/utils/makeSuspenseful.tsx
- src/components/ErrorBoundary/ErrorFallback.tsx
- src/components/ErrorPage/ErrorPageContent.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/ErrorBoundary/createErrorBoundary.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/ErrorBoundary/createErrorBoundary.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS wrap components in error boundaries
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS provide user-friendly fallback components in error boundaries
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS wrap components in error boundaries for production
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : Missing error boundaries in React components is an anti-pattern
Learnt from: gomesalexandre
PR: shapeshift/web#10220
File: src/pages/Explore/components/Tags.tsx:18-118
Timestamp: 2025-08-07T16:09:41.502Z
Learning: gomesalexandre indicated that error boundaries are not necessary for DOM manipulation in components like Tags. He prefers focusing error boundaries around suspense boundaries and implementing them more strategically as local error boundaries rather than wrapping every component.
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS log errors for debugging in error boundaries
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS provide user-friendly fallback components in error boundaries
Applied to files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : Missing error boundaries in React components is an anti-pattern
Applied to files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS wrap components in error boundaries
Applied to files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
📚 Learning: 2025-08-03T22:10:38.426Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/react-best-practices.mdc:0-0
Timestamp: 2025-08-03T22:10:38.426Z
Learning: Applies to **/*.{tsx,jsx} : ALWAYS wrap components in error boundaries for production
Applied to files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.tsx : ALWAYS log errors for debugging in error boundaries
Applied to files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
📚 Learning: 2025-08-07T16:09:41.502Z
Learnt from: gomesalexandre
PR: shapeshift/web#10220
File: src/pages/Explore/components/Tags.tsx:18-118
Timestamp: 2025-08-07T16:09:41.502Z
Learning: gomesalexandre indicated that error boundaries are not necessary for DOM manipulation in components like Tags. He prefers focusing error boundaries around suspense boundaries and implementing them more strategically as local error boundaries rather than wrapping every component.
Applied to files:
src/components/ErrorBoundary/createErrorBoundary.tsxsrc/components/ErrorBoundary/index.ts
🧬 Code graph analysis (1)
src/components/ErrorBoundary/createErrorBoundary.tsx (1)
src/lib/mixpanel/mixPanelSingleton.ts (1)
getMixPanel(9-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Call / Static
🔇 Additional comments (1)
src/components/ErrorBoundary/index.ts (1)
1-4: Barrel exports LGTM.Named exports, clear module surface, and consistent casing. This aligns with our convention to avoid default exports.
|
It got super messy trying to add a debug error tester. I think better to just add temporary monkey patch errors around the place when we want to test |
ee0a694 to
8adc434
Compare
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.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/pages/Markets/components/CardWithSparkline.tsx (1)
39-39: Nested interactive controls: Card rendered as Button wraps a fallback with a Button (known a11y pitfall)Rendering the
Cardas a native<button>(as={Button}) means the chart fallback’s Retry<Button>(insideChartErrorBoundary) becomes a button-inside-button, which is invalid HTML and can swallow clicks. A prior review surfaced this and the author marked it out of scope for this PR; echoing here only to avoid regressions.Low-impact mitigation options:
- Make the outer Card non-button but clickable (role="button", keyboard handlers), or
- Keep Card as is and adjust the fallback retry control to render as a non-button and stop event propagation.
Below is a non-breaking change confined to this file that removes the native button while keeping clickability.
- <Card height='100%' width='100%' borderRadius='xl' p={0} as={Button} onClick={handleClick}> + <Card + height='100%' + width='100%' + borderRadius='xl' + p={0} + role='button' + tabIndex={0} + onClick={handleClick} + onKeyDown={e => (e.key === 'Enter' || e.key === ' ') && handleClick()} + cursor='pointer' + >If you prefer to keep the Card as a native button, alternatively update the Retry control in
ChartErrorBoundarytoas='div'and calle.stopPropagation()in itsonClick(in that file).
🧹 Nitpick comments (3)
src/pages/Markets/components/CardWithSparkline.tsx (3)
83-92: De-duplicate the chart height constant to avoid drift between fallback and chart
'150px'is hardcoded twice: once in the boundaryheightand once inPriceChart.chartHeight. If one changes but not the other, the fallback and the chart will mismatch vertically.Consider a single local constant and reuse it in both places.
Apply this diff within the selected range:
- <ChartErrorBoundary height='150px'> + <ChartErrorBoundary height={CHART_HEIGHT}> <PriceChart assetId={assetId} timeframe={HistoryTimeframe.DAY} percentChange={changePercent24Hr} setPercentChange={noop} - chartHeight='150px' + chartHeight={CHART_HEIGHT} hideAxis={true} /> </ChartErrorBoundary>Add this just above the component (outside the selected range):
const CHART_HEIGHT = '150px'
83-92: Double-check color source vs. chart data for potential mismatch
PriceChartcolors the graph frompercentChangebut also computes its own percent change internally and callssetPercentChange. PassingpercentChange={changePercent24Hr}together withsetPercentChange={noop}means the color will always reflect market data’s 24h change, not what the chart data actually computes for the selected timeframe. Here timeframe is DAY, so it likely aligns, but if this component ever varies timeframe, color could drift.If you intend to always use the 24h market delta for color, this is fine. Otherwise, consider either:
- Letting
PriceChartdrive color by omittingpercentChangeand wiringsetPercentChangeto local state, or- Computing the percent change from the same series you feed the chart and passing that in.
21-24: Wrap the component with React.memo to avoid avoidable re-rendersThis component receives props and selects from the store; memoization helps keep Markets grids snappy when many cards render. It’s a small change and consistent with our guidelines.
Add this outside the selected range:
import { memo } from 'react'Then refactor the export:
type CardWithSparklineProps = { assetId: AssetId onClick: (assetId: AssetId) => void } const CardWithSparklineComponent: React.FC<CardWithSparklineProps> = ({ assetId, onClick }) => { // ...existing implementation... } export const CardWithSparkline = memo(CardWithSparklineComponent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (18)
src/Routes/RoutesCommon.tsx(9 hunks)src/assets/translations/en/main.json(1 hunks)src/components/AssetHeader/AssetChart.tsx(2 hunks)src/components/ErrorBoundary/ChartErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/ComponentErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/ErrorFallback.tsx(1 hunks)src/components/ErrorBoundary/SuspenseErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/TradingErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/createErrorBoundary.tsx(1 hunks)src/components/ErrorBoundary/index.ts(1 hunks)src/components/ErrorPage/ErrorPageContent.tsx(1 hunks)src/pages/Assets/Assets.tsx(2 hunks)src/pages/Dashboard/components/DashboardChart.tsx(2 hunks)src/pages/ErrorPage/ErrorPage.tsx(1 hunks)src/pages/Markets/components/CardWithSparkline.tsx(2 hunks)src/pages/Trade/StandaloneTrade.tsx(2 hunks)src/pages/Trade/tabs/TradeTab.tsx(2 hunks)src/utils/makeSuspenseful.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- src/pages/Trade/tabs/TradeTab.tsx
- src/pages/Assets/Assets.tsx
- src/components/ErrorBoundary/TradingErrorBoundary.tsx
- src/components/ErrorBoundary/ErrorFallback.tsx
- src/components/ErrorBoundary/ComponentErrorBoundary.tsx
- src/components/ErrorBoundary/SuspenseErrorBoundary.tsx
- src/pages/Trade/StandaloneTrade.tsx
- src/components/ErrorBoundary/ChartErrorBoundary.tsx
- src/pages/Dashboard/components/DashboardChart.tsx
- src/pages/ErrorPage/ErrorPage.tsx
- src/components/ErrorBoundary/createErrorBoundary.tsx
- src/Routes/RoutesCommon.tsx
- src/utils/makeSuspenseful.tsx
- src/components/ErrorPage/ErrorPageContent.tsx
- src/assets/translations/en/main.json
- src/components/AssetHeader/AssetChart.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/ErrorBoundary/index.tssrc/pages/Markets/components/CardWithSparkline.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/ErrorBoundary/index.tssrc/pages/Markets/components/CardWithSparkline.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/ErrorBoundary/index.tssrc/pages/Markets/components/CardWithSparkline.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/pages/Markets/components/CardWithSparkline.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/pages/Markets/components/CardWithSparkline.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: premiumjibles
PR: shapeshift/web#10361
File: src/pages/Markets/components/CardWithSparkline.tsx:83-92
Timestamp: 2025-08-25T23:32:13.836Z
Learning: In shapeshift/web PR #10361, premiumjibles considered the nested button accessibility issue (ChartErrorFallback retry Button inside Card rendered as Button in CardWithSparkline.tsx) out of scope for the error boundaries feature PR, consistent with deferring minor a11y improvements to follow-up PRs.
📚 Learning: 2025-08-25T23:32:13.836Z
Learnt from: premiumjibles
PR: shapeshift/web#10361
File: src/pages/Markets/components/CardWithSparkline.tsx:83-92
Timestamp: 2025-08-25T23:32:13.836Z
Learning: In shapeshift/web PR #10361, premiumjibles considered the nested button accessibility issue (ChartErrorFallback retry Button inside Card rendered as Button in CardWithSparkline.tsx) out of scope for the error boundaries feature PR, consistent with deferring minor a11y improvements to follow-up PRs.
Applied to files:
src/pages/Markets/components/CardWithSparkline.tsx
🧬 Code graph analysis (1)
src/pages/Markets/components/CardWithSparkline.tsx (3)
src/components/ErrorBoundary/ChartErrorBoundary.tsx (1)
ChartErrorBoundary(13-47)src/components/ErrorBoundary/index.ts (1)
ChartErrorBoundary(2-2)src/components/PriceChart/PriceChart.tsx (1)
PriceChart(31-109)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Call / Static
🔇 Additional comments (2)
src/pages/Markets/components/CardWithSparkline.tsx (1)
11-11: Good use of the barrel to import the chart boundaryImporting
ChartErrorBoundaryfrom the ErrorBoundary index keeps call sites clean and consistent across the app. No concerns here.src/components/ErrorBoundary/index.ts (1)
1-4: Barrel exports look good and align with usage across the appConsolidating boundaries behind a named-exports index improves discoverability and keeps imports uniform. No blocking issues.
Description
This introduces more granular error boundaries so that certain crashes don't crash the whole app. Adds it around sensitive areas like chart API's as well as suspense boundaries.
Issue (if applicable)
closes #10114
Risk
Low risk, just adds some error boundaries. Could look wierd during certain crashes.
Testing
Engineering
Revert the commit "remove error boundary testing"
localStorage.setItem('testTradingError', 'true')
Navigate to: /trade/fox (or any trade route)
localStorage.setItem('testPriceChartError', 'true')
Navigate to: /trade/fox (or any asset page with a chart)
localStorage.setItem('testBalanceChartError', 'true')
Navigate to: /dashboard
localStorage.setItem('testMarketsError', 'true')
Navigate to: /assets
localStorage.setItem('testSuspenseError', 'true')
Navigate to: /dashboard
Operations
Play around with the markets page, swapper, wallet/dashboard page and assets list on mobile and make sure all looks normal
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Localization