Skip to content

Commit

Permalink
feat: send common errors automatically, instead of asking user to rev…
Browse files Browse the repository at this point in the history
…iew (#1799)

- Closes #1678 
- Closes FE-1019

## Summary

- We now detect and handle common errors either by sending them directly
or ignoring them.

# Checklist

- [x] I've added error handling for all actions/requests.
- [x] I've reviewed how my changes will display in the UI.
- [x] I've checked all the copy (text) changes or additions in this PR.
- [x] I've included references to the issues being closed on GitHub
and/or Linear.
- [x] I've ensured that the documentation is up to date and reflects any
changes.
- [x] I've added documentation links where it may be helpful.
- [x] I **reviewed** the **entire PR** myself.

---------

Co-authored-by: LuizAsFight <[email protected]>
Co-authored-by: Luiz Gomes <[email protected]>
  • Loading branch information
3 people authored Feb 12, 2025
1 parent 20042d8 commit 118f4f4
Show file tree
Hide file tree
Showing 10 changed files with 317 additions and 237 deletions.
5 changes: 5 additions & 0 deletions .changeset/selfish-days-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"fuels-wallet": patch
---

feat: send common errors automatically, instead of asking user to review
2 changes: 1 addition & 1 deletion examples/cra-dapp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@
"@types/react-dom": "18.3.0",
"@vitejs/plugin-react": "4.2.1",
"typescript": "5.2.2",
"vite": "6.0.3"
"vite": "6.0.8"
}
}
73 changes: 55 additions & 18 deletions packages/app/playwright/e2e/ReportError.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,11 @@ test.describe('ReportError', () => {
await getByAriaLabel(page, 'Send error reports').click();
await expect(page.getByText(/Unexpected error/)).toHaveCount(0);

await page.waitForTimeout(2000);
const errorsAfterReporting = await getPageErrors(page);
expect(errorsAfterReporting.length).toBe(0);
await expect
.poll(async () => (await getPageErrors(page)).length, {
timeout: 10000,
})
.toBe(0);
});

test('should show Review Error in menu when there is a error in the database', async () => {
Expand All @@ -59,7 +61,7 @@ test.describe('ReportError', () => {
await window.fuelDB.errors.add({
id: '12345',
error: {
name: 'React error',
name: 'React$ error',
message: 'Test Error',
stack: 'Line error 1',
},
Expand All @@ -85,8 +87,8 @@ test.describe('ReportError', () => {
await window.fuelDB.errors.add({
id: '12345',
error: {
name: 'React error',
message: 'Test Error',
name: 'React$ error',
message: 'Test$ Error',
stack: 'Line error 1',
},
extra: {
Expand All @@ -103,20 +105,24 @@ test.describe('ReportError', () => {
page.locator(`[data-key="hasErrors"]`).click();
await hasText(page, /Unexpected error/i);

expect((await getPageErrors(page)).length).toBe(1);
// report error
await getByAriaLabel(page, 'Ignore error(s)').click();
await expect(page.getByText(/Unexpected error/i)).toHaveCount(0);

const errorsAfterReporting = await getPageErrors(page);
expect(errorsAfterReporting.length).toBe(1);
await expect
.poll(async () => (await getPageErrors(page)).length, {
timeout: 10000,
})
.toBe(1);
});
test('should be able to dismiss all errors', async () => {
await visit(page, '/');
await page.evaluate(async () => {
await window.fuelDB.errors.add({
id: '12345',
error: {
name: 'React error',
name: 'React$ error',
message: 'Test Error',
stack: 'Line error 1',
},
Expand All @@ -141,16 +147,19 @@ test.describe('ReportError', () => {
).click();
await expect(page.getByText(/Unexpected error/i)).toHaveCount(0);

const errorsAfterReporting = await getPageErrors(page);
expect(errorsAfterReporting.length).toBe(0);
await expect
.poll(async () => (await getPageErrors(page)).length, {
timeout: 10000,
})
.toBe(0);
});
test('should hide when the single error is dismissed', async () => {
await visit(page, '/');
await page.evaluate(async () => {
await window.fuelDB.errors.add({
id: '12345',
error: {
name: 'React error',
name: 'React$ error',
message: 'Test Error',
stack: 'Line error 1',
},
Expand All @@ -172,14 +181,17 @@ test.describe('ReportError', () => {
await getByAriaLabel(page, 'Dismiss error').click();
await expect(page.getByText(/Unexpected error/i)).toHaveCount(0);

const errorsAfterReporting = await getPageErrors(page);
expect(errorsAfterReporting.length).toBe(0);
await expect
.poll(async () => (await getPageErrors(page)).length, {
timeout: 10000,
})
.toBe(0);
});
test('should detect and capture global errors', async () => {
await visit(page, '/');
await page.evaluate(async () => {
console.error(new Error('Test Error'));
console.error(new Error('New Error'));
});
await visit(page, '/');
await reload(page);
await getByAriaLabel(page, 'Menu').click();
page.locator(`[data-key="hasErrors"]`).click();
Expand All @@ -195,7 +207,7 @@ test.describe('ReportError', () => {
await window.fuelDB.errors.add({
id: '12345',
error: {
name: 'React error',
name: 'React$ error',
message: 'Test Error',
stack: 'Line error 1',
},
Expand All @@ -210,7 +222,7 @@ test.describe('ReportError', () => {
await window.fuelDB.errors.add({
id: '123456',
error: {
name: 'React error',
name: 'React$ error',
message: 'Test Error',
stack: 'Line error 1',
},
Expand All @@ -231,4 +243,29 @@ test.describe('ReportError', () => {
const errorsAfterReporting = await getPageErrors(page);
expect(errorsAfterReporting.length).toBe(1);
});
test('should not show ignored errors', async () => {
await visit(page, '/');

await page.evaluate(async () => {
await window.fuelDB.errors.add({
id: '12345',
error: {
name: 'React Error',
message: 'React Error',
stack: 'Line error 1',
},
extra: {
timestamp: Date.now(),
location: 'http://localhost:3000',
pathname: '/',
hash: '#',
counts: 0,
},
});
});
await getByAriaLabel(page, 'Menu').click();
expect(
await page.locator(`[data-key="hasErrors"]`).isVisible()
).toBeFalsy();
});
});
Original file line number Diff line number Diff line change
@@ -1 +1 @@
export const BALANCE_NFTS_TAB_HEIGHT = 244;
export const BALANCE_NFTS_TAB_HEIGHT = 244;
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,9 @@ export const QuickAccountConnect = () => {
const onDismiss = () => {
if (!origin || !account) return;
setDismissed(true);
localStorage.setItem(getDismissKey(account.address, origin.full), 'true');
if (typeof localStorage !== 'undefined') {
localStorage.setItem(getDismissKey(account.address, origin.full), 'true');
}
};

useEffect(() => {
Expand Down
12 changes: 11 additions & 1 deletion packages/app/src/systems/Error/machines/reportErrorMachine.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { assign, createMachine } from 'xstate';

import { db } from '~/systems/Core/utils/database';
import { ErrorProcessorService } from '~/systems/Error/services/ErrorProcessorService';
import { getErrorIgnoreData } from '~/systems/Error/utils/getErrorIgnoreData';
import { ReportErrorService } from '../services';

export type ErrorMachineContext = {
Expand Down Expand Up @@ -124,6 +125,9 @@ export const reportErrorMachine = createMachine(
target: 'idle',
},
],
onError: {
target: 'idle',
},
},
},
reporting: {
Expand Down Expand Up @@ -175,7 +179,13 @@ export const reportErrorMachine = createMachine(
checkForErrors: async (context) => {
await context.errorProcessorService.processErrors();
const hasErrors = await context.reportErrorService.checkForErrors();
const errors = await context.reportErrorService.getErrors();
const errors = (await context.reportErrorService.getErrors()).filter(
(e) => !getErrorIgnoreData(e?.error)?.action
);
await context.reportErrorService.handleAndRemoveOldIgnoredErrors(
errors
);

return {
hasErrors,
errors,
Expand Down
69 changes: 48 additions & 21 deletions packages/app/src/systems/Error/services/ReportErrorService.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { StoredFuelWalletError } from '@fuel-wallet/types';
import * as Sentry from '@sentry/react';
import { db } from '~/systems/Core/utils/database';
import { captureException } from '~/systems/Error/utils/captureException';
import { getErrorIgnoreData } from '~/systems/Error/utils/getErrorIgnoreData';
import { parseFuelError } from '../utils';

export class ReportErrorService {
Expand All @@ -12,29 +13,32 @@ export class ReportErrorService {
if (typeof window !== 'undefined' && (window as any).playwright) {
return;
}
Sentry.captureException(e.error, {
extra: e.extra,
tags: { id: e.id, manual: true },
});
captureException(e.error, e.extra);
}
}

static async saveError(error: Error) {
const parsedError = parseFuelError(error);
if (!parsedError) {
console.warn(`Can't save error without a message`);
return;
}
if (!('id' in parsedError)) {
console.warn(`Can't save error without an id`);
return;
}
if (!db.isOpen() || db.hasBeenClosed()) {
console.warn('Error saving error: db is closed');
return;
}

try {
const parsedError = parseFuelError(error);
const ignoreData = getErrorIgnoreData(parsedError?.error);
if (!parsedError) {
console.warn(`Can't save error without a message`);
return;
}
if (!('id' in parsedError)) {
console.warn(`Can't save error without an id`);
return;
}
if (ignoreData?.action === 'ignore') return;
if (ignoreData?.action === 'hide') {
// Directly report to Sentry and exit
captureException(parsedError.error, parsedError.extra);
return;
}
if (!db.isOpen() || db.hasBeenClosed()) {
console.warn('Error saving error: db is closed');
return;
}
return await db.errors.add(parsedError);
} catch (e) {
console.warn('Failed to save error', e);
Expand All @@ -43,17 +47,40 @@ export class ReportErrorService {

async checkForErrors(): Promise<boolean> {
const errors = await this.getErrors();
return errors.length > 0;
return (
errors.filter((e) => !getErrorIgnoreData(e?.error)?.action).length > 0
);
}

async getErrors(): Promise<StoredFuelWalletError[]> {
return db.errors.toArray();
return await db.errors.toArray();
}

async clearErrors() {
await db.errors.clear();
}

async handleAndRemoveOldIgnoredErrors(errors: StoredFuelWalletError[]) {
const errorsBeingRemoved: Array<Promise<unknown>> = [];
// Convert to for of
for (const e of errors) {
const errorIgnoreData = getErrorIgnoreData(e?.error);
if (errorIgnoreData?.action) {
errorsBeingRemoved.push(
new Promise((resolve) => {
if (errorIgnoreData?.action === 'hide') {
captureException(e.error, e.extra);
}
return resolve(
this.dismissError(e.id).finally(() => resolve(true))
);
})
);
}
}
await Promise.all(errorsBeingRemoved);
}

async dismissError(key: string) {
if (!key) return;
db.errors.delete(key);
Expand Down
9 changes: 9 additions & 0 deletions packages/app/src/systems/Error/utils/captureException.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { SentryExtraErrorData } from '@fuel-wallet/types';
import * as Sentry from '@sentry/react';

export function captureException(error: Error, extra: SentryExtraErrorData) {
Sentry.captureException(error, {
extra,
tags: { manual: true },
});
}
Loading

0 comments on commit 118f4f4

Please sign in to comment.