getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
8.01k stars 1.58k forks source link

Sentry when running over capacitor as hybrid app stackrace is not having any information #1863

Closed stripathix closed 4 years ago

stripathix commented 5 years ago

Package + Version

Version:

"raven-js": "3.27.0",

Description

Code to install

Raven
                    .config(dsnToUse, {
                        release: "WSVue-" + AppConst.application.version + "-" + AppConst.application.bundleVersion,
                        environment: bootstrapAppService.getEnvironment(),
                        ignoreErrors: content.ignoreErrorsList,
                        tags: {appversion: appInfo.applicationVersion},
                        dataCallback: function (data) {
                            // do something to data
                            data.extra.localStorageData = getLocalStorageData();
                            return data;
                        }
                    })
                    .addPlugin(RavenVue, Vue)
                    .install();

With website environment it is working awesome. But I have same codebase as hybrid app using Capacitor.

Capacitor: https://capacitor.ionicframework.com/

Error infor logged is not having ang stack information.

Error data logged looks like this

{
  "project": "181355",
  "logger": "javascript",
  "platform": "javascript",
  "request": {
    "headers": {
      "User-Agent": "Mozilla/5.0 (iPhone; CPU iPhone OS 12_1 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/16B91"
    },
    "url": "capacitor://localhost#/app/developer"
  },
  "exception": {
    "values": [
      {
        "type": "Error",
        "value": "Forcing error fooboo tester",
        "stacktrace": {
          "frames": [
            {
              "filename": "[native code]",
              "lineno": null,
              "colno": null,
              "function": "Promise",
              "in_app": true
            },
            {
              "filename": "[native code]",
              "lineno": null,
              "colno": null,
              "function": "initializePromise",
              "in_app": true
            },
            {
              "filename": "[native code]",
              "lineno": null,
              "colno": null,
              "function": "Promise",
              "in_app": true
            },
            {
              "filename": "[native code]",
              "lineno": null,
              "colno": null,
              "function": "initializePromise",
              "in_app": true
            },
            {
              "filename": "[native code]",
              "lineno": null,
              "colno": null,
              "function": "forceError",
              "in_app": true
            }
          ]
        }
      }
    ],
    "mechanism": {
      "type": "generic",
      "handled": true
    }
  },
  "transaction": "[native code]",
  "trimHeadFrames": 0,
  "extra": {
    "message": "Forcing error fooboo tester",
    "code": "Forcing error fooboo tester.",
    "emailTried": "Forcing error fooboo. tester",
    "name": "Forcing error fooboo tester.",
    "session:duration": 66618
  },
  "tags": {
    "appversion": "4.4.6"
  },
  "breadcrumbs": {
    "values": [
      {
        "timestamp": 1549015935.474,
        "message": "Input: enableInputBlurring",
        "level": "debug",
        "category": "console"
      },
      {
        "timestamp": 1549015935.474,
        "message": "Input: enableScrollPadding",
        "level": "debug",
        "category": "console"
      },
      {
        "timestamp": 1549015935.481,
        "type": "http",
        "category": "xhr",
        "data": {
          "method": "GET",
          "url": "static/json/airports.json",
          "status_code": 200
        }
      },
      {
        "timestamp": 1549015935.745,
        "type": "http",
        "category": "xhr",
        "data": {
          "method": "GET",
          "url": "static/json/wx.json",
          "status_code": 200
        }
      },
      {
        "timestamp": 1549015935.759,
        "message": "checkUserAuthenticated",
        "level": "log",
        "category": "console"
      },
      {
        "timestamp": 1549015935.783,
        "message": "checkUserAuthenticated:Refreshing session",
        "level": "log",
        "category": "console"
      },
      {
        "timestamp": 1549015937.922,
        "type": "http",
        "category": "fetch",
        "data": {
          "method": "POST",
          "url": "https://cognito-idp.us-east-1.amazonaws.com/",
          "status_code": 200
        }
      },
      {
        "timestamp": 1549015938.318,
        "type": "http",
        "category": "fetch",
        "data": {
          "method": "POST",
          "url": "https://cognito-idp.us-east-1.amazonaws.com/",
          "status_code": 200
        }
      },
      {
        "timestamp": 1549015939.911,
        "type": "http",
        "category": "xhr",
        "data": {
          "method": "POST",
          "url": "https://cognito-identity.us-east-1.amazonaws.com/",
          "status_code": 200
        }
      }
    ]
  },
  "user": {
    "email": "********@gmail.com"
  },
  "environment": "DEV:CORDOVA:TFXC",
  "release": "WSVue-4.4.6-0.19.02.01.04.07",
  "event_id": "f137e408f8114bdcbc013be0b04f1f6a"
}

Stack information looks like this on sentry

screenshot 2019-02-01 at 6 01 19 pm

Same error logged on website looks very nice

52032491-2975c300-2547-11e9-8800-2e0b23bc69f6
stripathix commented 5 years ago

Hi @kamilogorek, could you please help with this.

stripathix commented 5 years ago

File name in stack frames looks wrong

{
              "filename": "[native code]",
              "lineno": null,
              "colno": null,
              "function": "forceError",
              "in_app": true
            }
stripathix commented 5 years ago

One more sample error console looks like this. Raven.js does not gets any information of error.

screenshot 2019-01-31 at 10 50 43 am

On sentry it looks like this

screenshot 2019-02-02 at 9 59 15 am
kamilogorek commented 5 years ago

It's TraceKit's responsiblity to extract proper frames info from the error itself – https://github.com/getsentry/sentry-javascript/blob/master/packages/raven-js/vendor/TraceKit/tracekit.js

We made some changes to it in new @sentry/browser SDK though, so you may want to give it a try first before we do any more investigation.

stripathix commented 5 years ago

@kamilogorek With @sentry/browser it's even worse that's why I moved to Raven.

Below is how it looks like when app is running as Hybrid app using capacitor + Ionic 4 + VueJS + @sentry/browser

screenshot 2019-02-07 at 10 32 20 am screenshot 2019-02-07 at 10 32 12 am

Below is the link of error which is logged when I use @sentry/browser.

https://sentry.io/share/issue/d0c7e9529ac94dba9c4b7b04e16361ba/

This is how the console looks like

screenshot 2019-02-07 at 11 41 18 am

Sentry Setup

Sentry.init({
    release: "WSVue-" + AppConst.application.version + "-" + AppConst.application.bundleVersion,
    dsn: "https://f5f708eca78141d1b09aec3c5eade8f1@sentry.io/181356",
    transport: Sentry.Transports.FetchTransport,
    integrations: [new Sentry.Integrations.Vue({Vue})],
    environment: "DEVTEST",
    beforeSend: function (exception) {
        return exception;
    }
});

Although it looks beautiful when running application is running as a website

screenshot 2019-02-07 at 10 30 44 am

Below is same error logged when app is running as website https://sentry.io/share/issue/4133deab3fc240f2bc85c680fae922a0/

kamilogorek commented 5 years ago

I never used Capacitor nor Ionic 4 tbh. Are you able to provide the smallest possible repro-case I could use to debug this? I don't have enough time to learn how to set it up.

stripathix commented 5 years ago

@kamilogorek Not a problem I have created a test project for reproducing the issue. Capacitor is exactly similar to cordova. Same debugging process.

https://github.com/stripathix/ionic-vue

To trigger an error just click on Trigger Error button

screenshot 2019-02-07 at 5 40 50 pm

To run the application I have added steps on README.md. It's pretty easy. Almost similar to Cordova environment.

To debug hybrid app it's exactly similar to Cordova debugging. Just run the app on the simulator and then from safari open Inspector for an app running on the simulator.

Below is the link of error logged when app was running a hybrid mobile app. https://sentry.io/share/issue/a19f3ecd72eb4e6fa216e4146ad91038/

screenshot 2019-02-07 at 5 44 22 pm
stripathix commented 5 years ago

hi, @kamilogorek did you got chance to look into this?

kamilogorek commented 5 years ago

Not really, I don't have much spare time these days. Will try to investigate it some day, sorry.

boozedog commented 5 years ago

@kamilogorek I too am experiencing this problem. Is there anyone else available who can look into this?

kamilogorek commented 5 years ago

Sorry, but I don't have enough time to tackle this issue right now.

If you'd like to investigate, I'd suggest to:

KevinKelchen commented 5 years ago

We use @sentry/browser in an Ionic Angular v4 app with Cordova and had similar issues with source-mapped stack traces.

We determined it had to do with the Ionic Web View for Cordova and its use of custom schemes on iOS (ionic:// for Cordova, capacitor:// for Capacitor). The custom scheme in the filenames of the frames of the stack trace seem problematic for deminification.

We wrote a workaround to rewrite the filenames' schemes from ionic:// to http:// and then it worked as expected. Here's a stripped-down version of it:

beforeSend = (event: Event) => {
  const exceptionValue = event.exception && event.exception.values && event.exception.values[0];

  // Approach taken from Sentry's React Native SDK.
  // See https://github.com/getsentry/sentry/issues/4719#issuecomment-333836573
  const stacktrace = event.stacktrace || (exceptionValue && exceptionValue.stacktrace);

  if (stacktrace) {
    stacktrace.frames.forEach(frame => {
      if (frame.filename !== "[native code]") {
        // The iOS webview uses a custom URL scheme for serving the web app.
        // See https://github.com/ionic-team/cordova-plugin-ionic-webview#iosscheme
        // The full URL with that scheme will be used for filenames in the stacktrace.
        // The URL begins with "ionic://localhost/".
        // The custom "ionic" scheme in the URL creates issues for source-mapped stacktraces.
        // Replacing "ionic" with "http" in the stackframe filename will enable source-mapped stacktraces.
        frame.filename = frame.filename.replace(/^ionic/, "http");
      }
    });
  }

  return event;
}

Hope this helps!

silviogutierrez commented 4 years ago

@KevinKelchen : this solves the incorrect filename, but how do you handle the undefined lineno and colno? Like @stripathix , the frames have those as undefined when running inside capacitor on an iOS web app. Example:

image

vs

image

Though to me it seems like it's not missing info but entire frames that are missing.

silviogutierrez commented 4 years ago

@kamilogorek : I can confirm originalException does have column and line info, see below:

image

Got no idea how to even start debugging tracekit, but maybe there's something obvious someone spots on the above.

kamilogorek commented 4 years ago

@silviogutierrez do you have any small repro that I could use to debug this? Ideally just a basic cordova app with frames that are missing col/line

silviogutierrez commented 4 years ago

@kamilogorek : I'll try to put one together, but I actually found the issue. It's quite simple, and exactly where you guessed it to be:

Right here: https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/tracekit.ts

The gecko and chrome variables both come back empty for the regex test on capacitor's frames. That is, when running on iOS, capacitor's frames have capacitor://localhost/foo/bar and on Android http://localhost. So Tracekit drops all frames in iOS except the ones with [native code] ones that match the regex. I would imagine it works fine on Android.

Simple running this (atrocious) sed command to change the gecko variable fixes the issue:

sed -i "s/moz-extension/moz-extension|capacitor/" node_modules/@sentry/browser/esm/tracekit.js

Should I open a PR against Tracekit to add the capacitor protocol? If you notice, the chrome regex is a little more flexible and already seems to have [-a-z] as part of the regex for any number of protocols. Should we try that in gecko too?

toddtarsi commented 4 years ago

@silviogutierrez - You ever get any resolution here? If need be, I can make a PR with your fix or even test it locally.

silviogutierrez commented 4 years ago

My fix worked. Unfortunately, I never had time to make a small reproduction repo with before/after. But I've been using the fix for a while successfully.

Two questions remain:

  1. Can this be made generic on sentry's side? Something like *:// for a protocol?
  2. Or instead, why does capacitor insist on its own protocol (and only on iOS)? Why not use one of the ones listed in tracekit.ts already?
toddtarsi commented 4 years ago

Those are good questions, and I don't have a very good answer for either.

  1. It seems like you have that figured out with your regex.

  2. I think that's just part of the whole Cordova / Ionic / Capacitor custom schemes thing or whatever. I feel like it also ties into the SSL pinning strategy to evade CORS preflighting from the app, but I'm not sure.

toddtarsi commented 4 years ago

@kamilogorek - I'm not really sure what you want for a repro. Here's my best shot I guess:

https://github.com/toddtarsi/cordova-create-react-app

I forked this repo, added Sentry, added a React error boundary, added a crash button. It's got Cordova run instructions. It's as baseline as you get with modern Cordova really. Let me know if this is what you're looking for.

toddtarsi commented 4 years ago

@kamilogorek @silviogutierrez - Okay, I'm done complaining here. I have come to the conclusion that the burden of this problem comes down to the brutally obtuse error implementation on the part of Gecko / safari. Thank you for being patient and helpful with me here.

kamilogorek commented 4 years ago

@toddtarsi sorry I didn't have enough time here, but thanks for letting me know. Let me close this issue, but feel free to ping me and/or open the new one if necessary. Thanks!

mrlowe commented 4 years ago

@kamilogorek would you like me to create a new issue as suggested to @toddtarsi, or just submit a PR referring to this one?

As pointed out in @silviogutierrez's comment above (https://github.com/getsentry/sentry-javascript/issues/1863#issuecomment-563364652) this is an issue particular to sentry-javascript's tweaked version of tracekit, and can easily be fixed in this project (https://github.com/getsentry/sentry-javascript/blob/master/packages/browser/src/tracekit.ts#L49).

Problem: Capacitor apps don't send mappable stacktraces in iOS because the gecko regex doesn't allow for their capacitor:// url scheme. Solution: Add the capacitor url scheme to the gecko regex.

I'm about to fork the project and make the change for my own use, if you'd like me to submit a pull request let me know!

seanwu1105 commented 4 years ago

@kamilogorek @mrlowe any updates for the pull request https://github.com/mrlowe/sentry-javascript/commit/3a09f918e0cbadfdbc1c11d6d779d4718d597fb0? It would be helpful for the capacitor support discussed in the issue https://github.com/getsentry/sentry/issues/13169.

mrlowe commented 4 years ago

@seanwu1105 nothing from me. I have this running in production now and haven't seen any problems.

albertinad commented 4 years ago

@mrlowe do you have plans to make a PR to make that change available to everyone on next release? We are facing the same issue when using Capacitor in a hybrid app in iOS, and this fix https://github.com/mrlowe/sentry-javascript/commit/3a09f918e0cbadfdbc1c11d6d779d4718d597fb0 works for us. Thanks!

mrlowe commented 4 years ago

@albertinad I didn't hear anything back from @kamilogorek so I'll just create the PR and refer to this issue. If it doesn't get picked up we can create a new issue I suppose.

kamilogorek commented 4 years ago

Sorry for the late response @mrlowe, it's just too easy to miss a mention in already closed issue. Will keep an eye on the PR, thanks!