Merge "UI: improve crash reporting" into main
diff --git a/python/tools/check_imports.py b/python/tools/check_imports.py
index 3634419..a114982 100755
--- a/python/tools/check_imports.py
+++ b/python/tools/check_imports.py
@@ -353,10 +353,18 @@
return result
+def remove_prefix(s, prefix):
+ return s[len(prefix):] if s.startswith(prefix) else s
+
+
+def remove_suffix(s, suffix):
+ return s[:-len(suffix)] if s.endswith(suffix) else s
+
+
def find_imports(path):
src = path
- src = src.removeprefix(UI_SRC_DIR)
- src = src.removesuffix('.ts')
+ src = remove_prefix(src, UI_SRC_DIR)
+ src = remove_suffix(src, '.ts')
directory, _ = os.path.split(src)
with open(path) as f:
s = f.read()
diff --git a/ui/src/base/logging.ts b/ui/src/base/logging.ts
index 945d96a..8387783 100644
--- a/ui/src/base/logging.ts
+++ b/ui/src/base/logging.ts
@@ -12,11 +12,21 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-import {SCM_REVISION, VERSION} from '../gen/perfetto_version';
+import {VERSION} from '../gen/perfetto_version';
-export type ErrorHandler = (err: string) => void;
+export type ErrorType = 'ERROR'|'PROMISE_REJ'|'OTHER';
+export interface ErrorStackEntry {
+ name: string; // e.g. renderCanvas
+ location: string; // e.g. frontend_bundle.js:12:3
+}
+export interface ErrorDetails {
+ errType: ErrorType;
+ message: string; // Uncaught StoreError: No such subtree: tracks,1374,state
+ stack: ErrorStackEntry[];
+}
-let errorHandler: ErrorHandler = (_: string) => {};
+export type ErrorHandler = (err: ErrorDetails) => void;
+const errorHandlers: ErrorHandler[] = [];
export function assertExists<A>(value: A | null | undefined): A {
if (value === null || value === undefined) {
@@ -35,34 +45,86 @@
assertTrue(!value, optMsg);
}
-export function setErrorHandler(handler: ErrorHandler) {
- errorHandler = handler;
+export function addErrorHandler(handler: ErrorHandler) {
+ if (!errorHandlers.includes(handler)) {
+ errorHandlers.push(handler);
+ }
}
export function reportError(err: ErrorEvent|PromiseRejectionEvent|{}) {
- let errLog = '';
let errorObj = undefined;
+ let errMsg = '';
+ let errType: ErrorType;
+ const stack: ErrorStackEntry[] = [];
+ const baseUrl = `${location.protocol}//${location.host}`;
if (err instanceof ErrorEvent) {
- errLog = err.message;
+ errType = 'ERROR';
+ errMsg = err.message;
errorObj = err.error;
} else if (err instanceof PromiseRejectionEvent) {
- errLog = `${err.reason}`;
+ errType = 'PROMISE_REJ';
+ errMsg = `PromiseRejection: ${err.reason}`;
errorObj = err.reason;
} else {
- errLog = `${err}`;
+ errType = 'OTHER';
+ errMsg = `Err: ${err}`;
}
if (errorObj !== undefined && errorObj !== null) {
- const errStack = (errorObj as {stack?: string}).stack;
- errLog += '\n';
- errLog += errStack !== undefined ? errStack : JSON.stringify(errorObj);
- }
- errLog += '\n\n';
- errLog += `${VERSION} ${SCM_REVISION}\n`;
- errLog += `UA: ${navigator.userAgent}\n`;
+ const maybeStack = (errorObj as {stack?: string}).stack;
+ let errStack = maybeStack !== undefined ? `${maybeStack}` : '';
+ errStack = errStack.replaceAll(/\r/g, ''); // Strip Windows CR.
+ for (let line of errStack.split('\n')) {
+ if (errMsg.includes(line)) continue;
+ // Chrome, Firefox and safari don't agree on the stack format:
+ // Chrome: prefixes entries with a ' at ' and uses the format
+ // function(https://url:line:col), e.g.
+ // ' at FooBar (https://.../frontend_bundle.js:2073:15)'
+ // however, if the function name is not known, it prints just:
+ // ' at https://.../frontend_bundle.js:2073:15'
+ // or also:
+ // ' at <anonymous>:5:11'
+ // Firefox and Safari: don't have any prefix and use @ as a separator:
+ // redrawCanvas@https://.../frontend_bundle.js:468814:26
+ // @debugger eval code:1:32
- console.error(errLog, err);
- errorHandler(errLog);
+ // Here we first normalize Chrome into the Firefox/Safari format by
+ // removing the ' at ' prefix and replacing (xxx)$ into @xxx.
+ line = line.replace(/^\s*at\s*/, '');
+ line = line.replace(/\s*\(([^)]+)\)$/, '@$1');
+
+ // This leaves us still with two possible options here:
+ // 1. FooBar@https://ui.perfetto.dev/v123/frontend_bundle.js:2073:15
+ // 2. https://ui.perfetto.dev/v123/frontend_bundle.js:2073:15
+ const lastAt = line.lastIndexOf('@');
+ let entryName = '';
+ let entryLocation = '';
+ if (lastAt >= 0) {
+ entryLocation = line.substring(lastAt + 1);
+ entryName = line.substring(0, lastAt);
+ } else {
+ entryLocation = line;
+ }
+
+ // Remove redundant https://ui.perfetto.dev/v38.0-d6ed090ee/ as we have
+ // that information already and don't need to repeat it on each line.
+ if (entryLocation.includes(baseUrl)) {
+ entryLocation = entryLocation.replace(baseUrl, '');
+ entryLocation = entryLocation.replace(`/${VERSION}/`, '');
+ }
+ stack.push({name: entryName, location: entryLocation});
+ }
+ }
+ // Invoke all the handlers registered through addErrorHandler.
+ // There are usually two handlers registered, one for the UI (error_dialog.ts)
+ // and one for Analytics (analytics.ts).
+ for (const handler of errorHandlers) {
+ handler({
+ errType,
+ message: errMsg,
+ stack,
+ } as ErrorDetails);
+ }
}
// This function serves two purposes.
diff --git a/ui/src/frontend/analytics.ts b/ui/src/frontend/analytics.ts
index 74c42fc..25f8e96 100644
--- a/ui/src/frontend/analytics.ts
+++ b/ui/src/frontend/analytics.ts
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+import {ErrorDetails} from '../base/logging';
import {getCurrentChannel} from '../common/channels';
import {VERSION} from '../gen/perfetto_version';
@@ -58,16 +59,16 @@
export interface Analytics {
initialize(): void;
updatePath(_: string): void;
- logEvent(_x: TraceCategories|null, _y: string): void;
- logError(_x: string, _y?: boolean): void;
+ logEvent(category: TraceCategories|null, event: string): void;
+ logError(err: ErrorDetails): void;
isEnabled(): boolean;
}
export class NullAnalytics implements Analytics {
initialize() {}
updatePath(_: string) {}
- logEvent(_x: TraceCategories|null, _y: string) {}
- logError(_x: string) {}
+ logEvent(_category: TraceCategories|null, _event: string) {}
+ logError(_err: ErrorDetails) {}
isEnabled(): boolean {
return false;
}
@@ -135,8 +136,24 @@
gtagGlobals.gtag('event', event, {event_category: category});
}
- logError(description: string, fatal = true) {
- gtagGlobals.gtag('event', 'exception', {description, fatal});
+ logError(err: ErrorDetails) {
+ let stack = '';
+ for (const entry of err.stack) {
+ const shortLocation = entry.location.replace('frontend_bundle.js', '$');
+ stack += `${entry.name}(${shortLocation}),`;
+ }
+ // Strip trailing ',' (works also for empty strings without extra checks).
+ stack = stack.substring(0, stack.length - 1);
+
+ gtagGlobals.gtag('event', 'exception', {
+ description: err.message,
+ error_type: err.errType,
+
+ // As per GA4 all field are restrictred to 100 chars.
+ // page_title is the only one restricted to 1000 chars and we use that for
+ // the full crash report.
+ page_location: `http://crash?/${encodeURI(stack)}`,
+ });
}
isEnabled(): boolean {
diff --git a/ui/src/frontend/error_dialog.ts b/ui/src/frontend/error_dialog.ts
index e94bfd7..008f1bc 100644
--- a/ui/src/frontend/error_dialog.ts
+++ b/ui/src/frontend/error_dialog.ts
@@ -14,30 +14,26 @@
import m from 'mithril';
-import {assertExists} from '../base/logging';
+import {assertExists, ErrorDetails} from '../base/logging';
import {EXTENSION_URL} from '../common/recordingV2/recording_utils';
import {TraceUrlSource} from '../common/state';
import {saveTrace} from '../common/upload_utils';
import {RECORDING_V2_FLAG} from '../core/feature_flags';
+import {VERSION} from '../gen/perfetto_version';
import {globals} from './globals';
import {showModal} from './modal';
import {isShareable} from './trace_attrs';
-// Never show more than one dialog per minute.
-const MIN_REPORT_PERIOD_MS = 60000;
+// Never show more than one dialog per 10s.
+const MIN_REPORT_PERIOD_MS = 10000;
let timeLastReport = 0;
-// Keeps the last ERR_QUEUE_MAX_LEN errors while the dialog is throttled.
-const queuedErrors = new Array<string>();
-const ERR_QUEUE_MAX_LEN = 10;
-
-export function maybeShowErrorDialog(errLog: string) {
- globals.logging.logError(errLog);
+export function maybeShowErrorDialog(err: ErrorDetails) {
const now = performance.now();
// Here we rely on the exception message from onCannotGrowMemory function
- if (errLog.includes('Cannot enlarge memory')) {
+ if (err.message.includes('Cannot enlarge memory')) {
showOutOfMemoryDialog();
// Refresh timeLastReport to prevent a different error showing a dialog
timeLastReport = now;
@@ -45,46 +41,37 @@
}
if (!RECORDING_V2_FLAG.get()) {
- if (errLog.includes('Unable to claim interface')) {
+ if (err.message.includes('Unable to claim interface')) {
showWebUSBError();
timeLastReport = now;
return;
}
- if (errLog.includes('A transfer error has occurred') ||
- errLog.includes('The device was disconnected') ||
- errLog.includes('The transfer was cancelled')) {
+ if (err.message.includes('A transfer error has occurred') ||
+ err.message.includes('The device was disconnected') ||
+ err.message.includes('The transfer was cancelled')) {
showConnectionLostError();
timeLastReport = now;
return;
}
}
- if (errLog.includes('(ERR:fmt)')) {
+ if (err.message.includes('(ERR:fmt)')) {
showUnknownFileError();
return;
}
- if (errLog.includes('(ERR:rpc_seq)')) {
+ if (err.message.includes('(ERR:rpc_seq)')) {
showRpcSequencingError();
return;
}
if (timeLastReport > 0 && now - timeLastReport <= MIN_REPORT_PERIOD_MS) {
- queuedErrors.unshift(errLog);
- if (queuedErrors.length > ERR_QUEUE_MAX_LEN) queuedErrors.pop();
console.log('Suppressing crash dialog, last error notified too soon.');
return;
}
timeLastReport = now;
-
- // Append queued errors.
- while (queuedErrors.length > 0) {
- const queuedErr = queuedErrors.shift();
- errLog += `\n\n---------------------------------------\n${queuedErr}`;
- }
-
- const errTitle = errLog.split('\n', 1)[0].substr(0, 80);
+ const errTitle = err.message.split('\n', 1)[0].substring(0, 80);
const userDescription = '';
let checked = false;
const engine = globals.getCurrentEngine();
@@ -98,13 +85,13 @@
checked = (ev.target as HTMLInputElement).checked;
if (checked && engine && engine.source.type === 'FILE') {
saveTrace(engine.source.file).then((url) => {
- const errMessage = createErrorMessage(errLog, checked, url);
+ const errMessage = createErrorMessage(err, checked, url);
renderModal(
errTitle, errMessage, userDescription, shareTraceSection);
return;
});
}
- const errMessage = createErrorMessage(errLog, checked);
+ const errMessage = createErrorMessage(err, checked);
renderModal(
errTitle, errMessage, userDescription, shareTraceSection);
},
@@ -118,7 +105,7 @@
}
renderModal(
errTitle,
- createErrorMessage(errLog, checked),
+ createErrorMessage(err, checked),
userDescription,
shareTraceSection);
}
@@ -171,20 +158,28 @@
engine.source.url !== undefined;
}
-function createErrorMessage(errLog: string, checked: boolean, url?: string) {
- let errMessage = '';
+function createErrorMessage(err: ErrorDetails, checked: boolean, url?: string) {
+ let msg = `UI: ${location.protocol}//${location.host}/${VERSION}\n\n`;
+
+ // Append the trace stack.
+ msg += `${err.message}\n`;
+ for (const entry of err.stack) {
+ msg += ` - ${entry.name} (${entry.location})\n`;
+ }
+ msg += '\n';
+
+ // Append the trace URL.
const engine = globals.getCurrentEngine();
if (checked && url !== undefined) {
- errMessage += `Trace: ${url}`;
+ msg += `Trace: ${url}\n`;
} else if (urlExists()) {
- errMessage +=
- `Trace: ${(assertExists(engine).source as TraceUrlSource).url}`;
+ msg += `Trace: ${(assertExists(engine).source as TraceUrlSource).url}\n`;
} else {
- errMessage += 'To assist with debugging please attach or link to the ' +
- 'trace you were viewing.';
+ msg += 'Trace: not available, please provide repro steps.\n';
}
- return errMessage + '\n\n' +
- 'Viewed on: ' + self.location.origin + '\n\n' + errLog;
+ msg += `UA: ${navigator.userAgent}\n`;
+ msg += `Referrer: ${document.referrer}\n`;
+ return msg;
}
function createLink(
diff --git a/ui/src/frontend/index.ts b/ui/src/frontend/index.ts
index 9ab3d09..581f61c 100644
--- a/ui/src/frontend/index.ts
+++ b/ui/src/frontend/index.ts
@@ -20,7 +20,7 @@
import m from 'mithril';
import {defer} from '../base/deferred';
-import {reportError, setErrorHandler} from '../base/logging';
+import {addErrorHandler, reportError} from '../base/logging';
import {Actions, DeferredAction, StateActions} from '../common/actions';
import {CommandManager} from '../common/commands';
import {createEmptyState} from '../common/empty_state';
@@ -205,8 +205,11 @@
document.head.append(script, css);
+ // Route errors to both the UI bugreport dialog and Analytics (if enabled).
+ addErrorHandler(maybeShowErrorDialog);
+ addErrorHandler((e) => globals.logging.logError(e));
+
// Add Error handlers for JS error and for uncaught exceptions in promises.
- setErrorHandler((err: string) => maybeShowErrorDialog(err));
window.addEventListener('error', (e) => reportError(e));
window.addEventListener('unhandledrejection', (e) => reportError(e));
diff --git a/ui/src/frontend/publish.ts b/ui/src/frontend/publish.ts
index d23be62..052cdea 100644
--- a/ui/src/frontend/publish.ts
+++ b/ui/src/frontend/publish.ts
@@ -153,7 +153,6 @@
export function publishMetricError(error: string) {
globals.setMetricError(error);
- globals.logging.logError(error, false);
globals.publishRedraw();
}
diff --git a/ui/src/frontend/trace_converter.ts b/ui/src/frontend/trace_converter.ts
index a08f622..fb0bc12 100644
--- a/ui/src/frontend/trace_converter.ts
+++ b/ui/src/frontend/trace_converter.ts
@@ -13,6 +13,7 @@
// limitations under the License.
import {download} from '../base/clipboard';
+import {ErrorDetails} from '../base/logging';
import {time} from '../base/time';
import {Actions} from '../common/actions';
import {
@@ -51,7 +52,7 @@
interface ErrorArgs {
kind: 'error';
- error: string;
+ error: ErrorDetails;
}
diff --git a/ui/src/traceconv/index.ts b/ui/src/traceconv/index.ts
index 627df91..d9c6aed 100644
--- a/ui/src/traceconv/index.ts
+++ b/ui/src/traceconv/index.ts
@@ -13,7 +13,12 @@
// limitations under the License.
import {defer} from '../base/deferred';
-import {assertExists, reportError, setErrorHandler} from '../base/logging';
+import {
+ addErrorHandler,
+ assertExists,
+ ErrorDetails,
+ reportError,
+} from '../base/logging';
import {time} from '../base/time';
import {
ConversionJobName,
@@ -60,7 +65,7 @@
});
}
-function forwardError(error: string) {
+function forwardError(error: ErrorDetails) {
selfWorker.postMessage({
kind: 'error',
error,
@@ -221,7 +226,7 @@
selfWorker.onmessage = (msg: MessageEvent) => {
self.addEventListener('error', (e) => reportError(e));
self.addEventListener('unhandledrejection', (e) => reportError(e));
- setErrorHandler((err: string) => forwardError(err));
+ addErrorHandler((error: ErrorDetails) => forwardError(error));
const args = msg.data as Args;
if (isConvertTraceAndDownload(args)) {
ConvertTraceAndDownload(args.trace, args.format, args.truncate);
