-
Notifications
You must be signed in to change notification settings - Fork 226
Add CI test job and global error boundary for webview-app #1957
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
base: dev
Are you sure you want to change the base?
Changes from all commits
ac0c88d
b5e649d
ab9d1c1
a97c67b
99c610a
aa0704e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| // SPDX-FileCopyrightText: 2025-2026 Social Connect Labs, Inc. | ||
| // SPDX-License-Identifier: BUSL-1.1 | ||
| // NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. | ||
|
|
||
| import type React from 'react'; | ||
| import { Component } from 'react'; | ||
|
|
||
| import { bridgeLifecycleAdapter } from '@selfxyz/webview-bridge/adapters'; | ||
|
|
||
| import { useBridge } from '../providers/BridgeProvider'; | ||
|
|
||
| interface ErrorBoundaryProps { | ||
| children: React.ReactNode; | ||
| onDismiss?: () => void; | ||
| } | ||
|
|
||
| interface ErrorBoundaryState { | ||
| hasError: boolean; | ||
| error: Error | null; | ||
| } | ||
|
|
||
| class ErrorBoundaryInner extends Component<ErrorBoundaryProps, ErrorBoundaryState> { | ||
| state: ErrorBoundaryState = { hasError: false, error: null }; | ||
|
|
||
| static getDerivedStateFromError(error: Error): ErrorBoundaryState { | ||
| return { hasError: true, error }; | ||
| } | ||
|
|
||
| componentDidCatch(error: Error, info: React.ErrorInfo): void { | ||
| if (import.meta.env.DEV) { | ||
| console.error('[ErrorBoundary] Unhandled error:', error, info.componentStack); | ||
| } else { | ||
| console.error('[ErrorBoundary] Unhandled error'); | ||
| } | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| handleRetry = (): void => { | ||
| this.setState({ hasError: false, error: null }); | ||
| }; | ||
|
|
||
| handleDismiss = (): void => { | ||
| this.props.onDismiss?.(); | ||
| }; | ||
|
|
||
| render(): React.ReactNode { | ||
| if (!this.state.hasError) { | ||
| return this.props.children; | ||
| } | ||
|
|
||
| return ( | ||
| <div style={styles.container}> | ||
| <div style={styles.content}> | ||
| <div style={styles.icon}>!</div> | ||
| <h1 style={styles.title}>Something went wrong</h1> | ||
| <p style={styles.message}>An unexpected error occurred. You can try again or close this screen.</p> | ||
| {import.meta.env.DEV && this.state.error && <pre style={styles.errorDetail}>{this.state.error.message}</pre>} | ||
| <button type="button" onClick={this.handleRetry} style={styles.primaryButton}> | ||
| Try again | ||
| </button> | ||
| <button type="button" onClick={this.handleDismiss} style={styles.secondaryButton}> | ||
| Close | ||
| </button> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export const ErrorBoundary: React.FC<{ children: React.ReactNode }> = ({ children }) => { | ||
| const bridge = useBridge(); | ||
|
|
||
| const handleDismiss = (): void => { | ||
| const lifecycle = bridgeLifecycleAdapter(bridge); | ||
| lifecycle.dismiss({ reason: 'user_cancel' }); | ||
| }; | ||
|
|
||
| return <ErrorBoundaryInner onDismiss={handleDismiss}>{children}</ErrorBoundaryInner>; | ||
| }; | ||
|
|
||
| const styles = { | ||
| container: { | ||
| display: 'flex', | ||
| flexDirection: 'column' as const, | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| height: '100vh', | ||
| width: '100%', | ||
| padding: 24, | ||
| backgroundColor: '#fff', | ||
| fontFamily: '-apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif', | ||
| }, | ||
| content: { | ||
| display: 'flex', | ||
| flexDirection: 'column' as const, | ||
| alignItems: 'center', | ||
| maxWidth: 320, | ||
| textAlign: 'center' as const, | ||
| }, | ||
| icon: { | ||
| width: 48, | ||
| height: 48, | ||
| borderRadius: 24, | ||
| backgroundColor: '#FEE2E2', | ||
| color: '#DC2626', | ||
| display: 'flex', | ||
| alignItems: 'center', | ||
| justifyContent: 'center', | ||
| fontSize: 24, | ||
| fontWeight: 700, | ||
| marginBottom: 16, | ||
| }, | ||
| title: { | ||
| fontSize: 20, | ||
| fontWeight: 600, | ||
| color: '#111', | ||
| margin: '0 0 8px', | ||
| }, | ||
| message: { | ||
| fontSize: 14, | ||
| color: '#666', | ||
| lineHeight: 1.5, | ||
| margin: '0 0 24px', | ||
| }, | ||
| errorDetail: { | ||
| fontSize: 11, | ||
| color: '#999', | ||
| backgroundColor: '#f5f5f5', | ||
| borderRadius: 8, | ||
| padding: 12, | ||
| width: '100%', | ||
| overflow: 'auto' as const, | ||
| maxHeight: 80, | ||
| marginBottom: 24, | ||
| textAlign: 'left' as const, | ||
| }, | ||
| primaryButton: { | ||
| width: '100%', | ||
| padding: '14px 24px', | ||
| fontSize: 16, | ||
| fontWeight: 600, | ||
| color: '#fff', | ||
| backgroundColor: '#111', | ||
| border: 'none', | ||
| borderRadius: 12, | ||
| cursor: 'pointer', | ||
| marginBottom: 12, | ||
| }, | ||
| secondaryButton: { | ||
| width: '100%', | ||
| padding: '14px 24px', | ||
| fontSize: 16, | ||
| fontWeight: 600, | ||
| color: '#666', | ||
| backgroundColor: 'transparent', | ||
| border: '1px solid #ddd', | ||
| borderRadius: 12, | ||
| cursor: 'pointer', | ||
| }, | ||
| }; | ||
|
Comment on lines
+80
to
+159
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,91 @@ | ||||||||||||||||||||||||||||||||
| // SPDX-FileCopyrightText: 2025-2026 Social Connect Labs, Inc. | ||||||||||||||||||||||||||||||||
| // SPDX-License-Identifier: BUSL-1.1 | ||||||||||||||||||||||||||||||||
| // NOTE: Converts to Apache-2.0 on 2029-06-11 per LICENSE. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // @vitest-environment jsdom | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import type React from 'react'; | ||||||||||||||||||||||||||||||||
| import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import { cleanup, fireEvent, render, screen } from '@testing-library/react'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const dismissMock = vi.fn(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| vi.mock('../../src/providers/BridgeProvider', () => ({ | ||||||||||||||||||||||||||||||||
| useBridge: () => ({}), | ||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| vi.mock('@selfxyz/webview-bridge/adapters', () => ({ | ||||||||||||||||||||||||||||||||
| bridgeLifecycleAdapter: () => ({ | ||||||||||||||||||||||||||||||||
| dismiss: dismissMock, | ||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // Import after mocks | ||||||||||||||||||||||||||||||||
| const { ErrorBoundary } = await import('../../src/components/ErrorBoundary'); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| const ThrowingChild: React.FC<{ shouldThrow: boolean }> = ({ shouldThrow }) => { | ||||||||||||||||||||||||||||||||
| if (shouldThrow) throw new Error('test crash'); | ||||||||||||||||||||||||||||||||
| return <div>healthy content</div>; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| describe('ErrorBoundary', () => { | ||||||||||||||||||||||||||||||||
| beforeEach(() => { | ||||||||||||||||||||||||||||||||
| vi.clearAllMocks(); | ||||||||||||||||||||||||||||||||
| vi.spyOn(console, 'error').mockImplementation(() => {}); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| afterEach(cleanup); | ||||||||||||||||||||||||||||||||
|
Comment on lines
+33
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Restore Line 35 installs a global spy, but Line 38 does not restore it. This can leak mocked console behavior into subsequent tests. Suggested fix- afterEach(cleanup);
+ afterEach(() => {
+ cleanup();
+ vi.restoreAllMocks();
+ });📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| it('renders children when no error', () => { | ||||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||||
| <ErrorBoundary> | ||||||||||||||||||||||||||||||||
| <ThrowingChild shouldThrow={false} /> | ||||||||||||||||||||||||||||||||
| </ErrorBoundary>, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| expect(screen.getByText('healthy content')).toBeDefined(); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| it('catches errors and shows fallback UI', () => { | ||||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||||
| <ErrorBoundary> | ||||||||||||||||||||||||||||||||
| <ThrowingChild shouldThrow={true} /> | ||||||||||||||||||||||||||||||||
| </ErrorBoundary>, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
| expect(screen.getByText('Something went wrong')).toBeDefined(); | ||||||||||||||||||||||||||||||||
| expect(screen.getByRole('button', { name: /try again/i })).toBeDefined(); | ||||||||||||||||||||||||||||||||
| expect(screen.getByRole('button', { name: /close/i })).toBeDefined(); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| it('resets error state when retry is clicked', () => { | ||||||||||||||||||||||||||||||||
| let shouldThrow = true; | ||||||||||||||||||||||||||||||||
| const Conditional: React.FC = () => { | ||||||||||||||||||||||||||||||||
| if (shouldThrow) throw new Error('test crash'); | ||||||||||||||||||||||||||||||||
| return <div>recovered</div>; | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||||
| <ErrorBoundary> | ||||||||||||||||||||||||||||||||
| <Conditional /> | ||||||||||||||||||||||||||||||||
| </ErrorBoundary>, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| expect(screen.getByText('Something went wrong')).toBeDefined(); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| shouldThrow = false; | ||||||||||||||||||||||||||||||||
| fireEvent.click(screen.getByRole('button', { name: /try again/i })); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| expect(screen.getByText('recovered')).toBeDefined(); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| it('calls dismiss on close', () => { | ||||||||||||||||||||||||||||||||
| render( | ||||||||||||||||||||||||||||||||
| <ErrorBoundary> | ||||||||||||||||||||||||||||||||
| <ThrowingChild shouldThrow={true} /> | ||||||||||||||||||||||||||||||||
| </ErrorBoundary>, | ||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| fireEvent.click(screen.getByRole('button', { name: /close/i })); | ||||||||||||||||||||||||||||||||
| expect(dismissMock).toHaveBeenCalledWith({ reason: 'user_cancel' }); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||
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.
Missing required CI fast-fail guard before test execution
Line 68 onward adds the new test job, but it does not include the required pre-test validation step to detect nested
require()patterns. This can reintroduce avoidable OOM-style CI failures later in the job.As per coding guidelines: ".github/workflows/*.yml: Add a CI fast-fail check step that runs the validation script to detect nested require() patterns before test execution to prevent out-of-memory pipeline failures."