getsentry / sentry-capacitor

The official Sentry SDK for Capacitor
https://sentry.io
MIT License
119 stars 33 forks source link

Object normalization should respect `normalizeDepth` and `normalizeMaxBreadth` configuration options #758

Open DavidStrausz opened 2 weeks ago

DavidStrausz commented 2 weeks ago

Logging complex/circular objects may lead to a webview/app crash.

Environment

Steps to Reproduce

I was logging Angular component instances to the console inside a signal effect for debugging purposes.

export class SelectComponent<T> {
  private options = contentChildren<SelectOptionComponent<T>>(
    SelectOptionComponent,
  );

  constructor() {
    effect(
      () => {
        console.log('This will cause the native app to crash', this.options());
      },
    );
}

Expected Result

The convertToNormalizedObject function should respect the normalizeDepth and normalizeMaxBreadth configuration options.

After I realized what was causing the problem, I hardcoded a max depth of 10 in the convertToNormalizedObject function, which worked, and avoided the crash. Later I found that there is a configuration option in sentry javascript which is unfortunately not respected by sentry capacitor.

Actual Result

When running on a native device (Android in my case) sentry capacitor tries to normalize the logged objects to add them as breadcrumbs, this blocks the main thread long enough for the OS to kill the webview process and subsequently the entire app. By default the normalize function from @sentry/utils traverses objects to a max depth of 100, which seems to be too much in this particular case. The exception is quite cryptic:

2024-10-17 15:12:28.936  7431-7431  Capacitor/Console       com.app.id      I  File: https://localhost/chunk-5KY5ZYNK.js - Line 1049 - Msg: This will cause the native app to crash 
2024-10-17 15:12:28.936  7431-7431  Capacitor/Plugin        com.app.id      V  To native (Capacitor plugin): callbackId: 95065978, pluginId: SentryCapacitor, methodName: addBreadcrumb
2024-10-17 15:12:28.937  7431-7431  Capacitor               com.app.id      V  callback: 95065978, pluginId: SentryCapacitor, methodName: addBreadcrumb, methodData: {"timestamp":1.729170748923E9,"category":"console","data":{"arguments":["This will cause the native app to crash",[]],"logger":"console"},"level":"debug","message":"This will cause the native app to crash "}
2024-10-17 15:12:35.005  7431-7431  Choreographer           com.app.id      I  Skipped 161 frames!  The application may be doing too much work on its main thread.
2024-10-17 15:13:07.320  7431-7431  Choreographer           com.app.id      I  Skipped 1607 frames!  The application may be doing too much work on its main thread.
2024-10-17 15:13:14.890  7431-7595  cr_ChildProcessConn     com.app.id      W  onServiceDisconnected (crash or killed by oom): pid=7599 bindings:W S
2024-10-17 15:13:14.900  7431-7431  chromium                com.app.id      I  [INFO:render_frame_host_impl.cc(11505)] RenderFrameHostImpl::MaybeGenerateCrashReport url = https://localhost/, status = 0, exit_code = 0
2024-10-17 15:13:14.918  7431-7431  chromium                com.app.id      E  [ERROR:aw_browser_terminator.cc(156)] Renderer process (7599) crash detected (code 5).
2024-10-17 15:13:14.924  7431-7431  chromium                com.app.id      A  [FATAL:crashpad_client_linux.cc(724)] Render process (7599)'s crash wasn't handled by all associated  webviews, triggering application crash.
2024-10-17 15:13:14.999  7431-7431  libc                    com.app.id      A  Fatal signal 5 (SIGTRAP), code 1 (TRAP_BRKPT), fault addr 0x70f63b3aa0 in tid 7431 (.app.id), pid 7431 (.app.id)
2024-10-17 15:13:15.282  7919-7919  DEBUG                   pid-7919                             A  Cmdline: com.app.id
2024-10-17 15:13:15.282  7919-7919  DEBUG                   pid-7919                             A  pid: 7431, tid: 7431, name: .app.id  >>> com.app.id <<<
---------------------------- PROCESS ENDED (7431) for package com.app.id ----------------------------
lucas-zimerman commented 1 week ago

Hi and thank you for opening this issue! Indeed, this function should respect the given options set by the user, we'll fix it!