Grouping still occasionally wrong

[edenman]

Expected behavior

different stacktraces are grouped into different errors

Observed behavior

Two totally different stacktraces are grouped into the same error:
https://app.bugsnag.com/recharge-labs/ios/errors/5a0226b08110ee00185760ea?&event_id=5a5681dde2a9ff00189926dc
https://app.bugsnag.com/recharge-labs/ios/errors/5a0226b08110ee00185760ea?&event_id=5a541f0ed0f0c0001cf4853b

Version

5.14.0

[fractalwrench]

I've discussed this with the backend team and they've confirmed that this grouping has occurred due to the similar lines on the top frame: (file: _hidden#23282/_hidden#23281, line: 134).

__hidden is normally present in a symbolicated stackframe if a project is using bitcode, and symbols have not been applied to the dSYM. We've written a bit of an explanation here.

One possible cause is that the top frame is referring to a dSYM other than the main app dSYM. Does your app use any frameworks or 3rd party libraries which could be producing this?

[edenman]

Thanks for the investigation, @fractalwrench!

1. Usually when there are missing dSYMs, the issue has a banner at the top that says "upload your dSYMs", right? I've definitely uploaded dSYMs for the release in question.
2. Shouldn't the rest of the stacktrace be taken into consideration when grouping?
3. In both cases, the method that is being called is a helper method that I wrote that lives in my main codebase:

extension Optional {
  func require(error: String) -> Wrapped {
    if self == nil {
      fatalError(error)
    }
    return self!
  }
}

[snmaynard]

I believe the dSYM isnt missing - it just has hidden symbols in it due to bitcode. You can look at the troubleshooting-hidden-symbols section of the link that @fractalwrench pasted above. For some reason the symbol is not in your dsym and is marked as hidden.

[edenman]

Sorry, totally glazed over that part. I'm using Xcode 9.2 (9C40b) and I'm leaving the "Include bitcode for iOS content" checkbox checked when I upload builds. I'll try running bugsnag-dsym-upload with the symbol map param and see if that un-hides the symbol.

Regardless, my question #2 from above seems to be the relevant one here, yeah? Shouldn't the whole stacktrace be taken into account, instead of just the top symbol?

[snmaynard]

We don't group on the whole stacktrace because most of the time that's not what you want. If you have a helper function that has a bug in it you don't want a group created for every call of that helper function, you want a single group instead.

The only cases I've seen it break down is where you have error helper functions that overgroups things together (like a logging function or something), and we typically recommend removing those frames from the error report in a beforeNotify function - if that is appropriate in this case I can show you how to do that.

[edenman]

Gotcha. I'm not sure I agree about that behavior being the default, but certainly if you can show me how to exclude this helper method with a beforeNotify block, that would be great :)

[snmaynard]

You can use the depth attribute. Details at https://docs.bugsnag.com/platforms/ios/reporting-handled-exceptions/#depth

You probably want to increment the depth by 1 to remove your function.

[edenman]

Ok so it looks like BugsnagBeforeSendBlock is what I want...but I don't want to globally add 1 to depth, right? How do I check if the crash is in this particular helper method?

[edenman]

Here's what I've got so far, and it seems to work at removing the helper method from the stacktrace:

      let beforeSend: BugsnagBeforeSendBlock = { rawEventData, reports in
        if let crash = rawEventData["crash"] as? NSDictionary,
          let threads = crash["threads"] as? NSArray,
          let mainThread = threads[0] as? NSDictionary,
          let notableAddresses = mainThread["notableAddresses"] as? NSDictionary {

          for key in notableAddresses.allKeys {
            if let addresses = notableAddresses[key] as? NSDictionary,
              let value = addresses["value"] as? String,
              value.hasSuffix("MyHelperClassForCrashing.swift") {

              // Found my helper method, chop it off the top of the stacktrace.
              reports.depth = 1
              break
            }
          }
        }
        return true
      }
      Bugsnag.configuration()?.add(beforeSend: beforeSend)

Unfortunately, it also seems to break grouping in a different way. Even when calling the helper method from the same place, I now get different Busgnag errors for each one:
https://app.bugsnag.com/recharge-labs/ios/errors/5a5fcb49aba85300189bef8c
https://app.bugsnag.com/recharge-labs/ios/errors/5a5fcc4c4c26c8001974a4ea

[edenman]

Also, I switched to a release build to test some of this stuff out, and now it seems that notableAddresses is no longer populated. Is there some other way I can figure out which file/class/method crashed?

[edenman]

Here's what I ended up with:

extension Bugsnag {
  static func dontUseSymbolForGrouping(_ symbol: String) {
    let beforeSend: BugsnagBeforeSendBlock = { rawEventData, reports in
      dbg("got rawEventData")
      guard let crash = rawEventData["crash"] as? NSDictionary else {
        dbg("no crash")
        return true
      }
      guard let threads = crash["threads"] as? NSArray else {
        dbg("no threads")
        return true
      }
      for (i, thread) in threads.enumerated() {
        guard let thread = thread as? NSDictionary else {
          dbg("thread \(i) is not a dictionary?!?")
          continue
        }
        guard let backtrace = thread["backtrace"] as? NSDictionary else {
          dbg("no backtrace for thread \(i)")
          continue
        }
        guard let backtraceContents = backtrace["contents"] as? NSArray else {
          dbg("no backtrace.contents for thread \(i)")
          continue
        }
        if let index = indexOfRequiredMethod(backtraceContents: backtraceContents, threadIndex: i, symbol: symbol) {
          DDLogDebug("Found 'require' method in crash stacktrace: chop it off so this is grouped by caller")
          reports.depth = UInt(index + 1)
          return true
        }
      }
      return true
    }
    Bugsnag.configuration()?.add(beforeSend: beforeSend)
  }
  
  private static func indexOfRequiredMethod(backtraceContents: NSArray, threadIndex: Int, symbol: String) -> Int? {
    for (i, stackFrame) in backtraceContents.enumerated() {
      guard let stackFrameDict = stackFrame as? NSDictionary else {
        dbg("stacktrace frame is not a dictionary? for thread \(threadIndex)")
        continue
      }
      if let symbolName = stackFrameDict["symbol_name"] as? String, symbolName == symbol {
        return i
      }
    }
    return nil
  }
}

I also had to add a @inline(never) to my require method to make sure it showed up in the backtrace.

In my case, symbol is passed in as _T0Sq8RechargeE7requirexSS5error_tFSS_Tg5Tf4gXn_nTf4nnnn_g but this seems super-fragile and I'd love to hear if you have ideas of how to do this better.

[fractalwrench]

@edenman the notableAddresses field error message changed in the latest version of Swift, the following commit should fix that issue.

We're currently planning on adding a new property to BugsnagCrashReport which would allow user-specified frames to be dropped from the stacktrace immediately. This would effectively replace depth.

[edenman]

@fractalwrench any progress on the user-specified frames thing? I just got bit by the backtrace symbol format being as fragile as I anticipated: it changed recently (in the latest Xcode/Swift?) and all my crashes from this helper method started getting lumped together again. Which meant I didn't notice them for a week.

[snmaynard]

We have some ideas on how we might solve it, but haven't had the time to focus on that for a while. Hopefully we will soon!

[edenman]

@snmaynard any updates on this? Swift4.2/Xcode10 broke my symbol detection code again.

[edenman]

Actually it looks like it was broken this time not because the symbol names changed in some subtle way, but instead that there don't seem to be any of my app's symbol names in the [crash][threads][backtrace][contents] array. This is really bad.

[edenman]

object_name.symbol_name from the TestFlight build:

libswiftCore.dylib._stdlib_destroyTLS 
libswiftCore.dylib._stdlib_destroyTLS 
Consumer._mh_execute_header
Consumer._mh_execute_header
Consumer._mh_execute_header
Consumer._mh_execute_header
Foundation.__NSFireDelayedPerform
CoreFoundation.<redacted>
CoreFoundation.<redacted>
CoreFoundation.<redacted>
CoreFoundation.<redacted>
CoreFoundation.CFRunLoopRunSpecific
GraphicsServices.GSEventRunModal
UIKitCore.UIApplicationMain
Consumer._mh_execute_header
libdyld.dylib.<redacted>

object_name.symbol_name from a dev build:

libswiftCore.dylib.$Ss17_assertionFailure__4file4line5flagss5NeverOs12StaticStringV_SSAHSus6UInt32VtFTf4nxnnn_n
libswiftCore.dylib.$Ss17_assertionFailure__4file4line5flagss5NeverOs12StaticStringV_SSAHSus6UInt32VtFTf4nxnnn_n
libswiftCore.dylib.$Ss17_assertionFailure__4file4line5flagss5NeverOs12StaticStringV_SSAHSus6UInt32VtF
Consumer.$S8Consumer10forceCrashys5NeverOSSF
Consumer.$S8Consumer14PaymentActionsC12addPromoCodeyyFZ
Consumer.$S8Consumer24PromotionsViewControllerC15addButtonTappedyyFySbcfU0_
Consumer.$SSbIegy_SbIeyBy_TR
Foundation.__NSFireDelayedPerform
CoreFoundation.<redacted>
CoreFoundation.<redacted>
CoreFoundation.<redacted>
CoreFoundation.<redacted>
CoreFoundation.CFRunLoopRunSpecific
GraphicsServices.GSEventRunModal
UIKitCore.UIApplicationMain
Consumer.main
libdyld.dylib.<redacted>

Every actual method in my app has just been replaced with _mh_execute_header in the kscrash report. What's weird is that the Bugsnag server is somehow able to map from that back to the actual method name (presumably from the symbol address?).

[kattrali]

Every actual method in my app has just been replaced with _mh_execute_header in the kscrash report. What's weird is that the Bugsnag server is somehow able to map from that back to the actual method name (presumably from the symbol address?).

Correct, optimizations are removing the symbol names at runtime in release builds, though uploaded reports are symbolicated using dsym/dylib files.

There is not much we can do to restore the old behavior of the compilation process, though you can potentially be clever about addresses instead of names to strip contents from reports. Here is an example, extending your previous code, which strips everything above a predefined closure which is used to report exceptions to Bugsnag. With a bit of elbow grease it could work for something like your use case, though yours is complicated by generics.

[edenman]

Ok, I'm finally getting around to looking into this. A couple questions that maybe you know the answer to:

1. Why does your code snippet do the rawEventData["user"] dance? Couldn't you just always check for callbackAddr inside the BugsnagBeforeSendBlock?
2. Any idea how to get unsafeBitCast on a generic method? I tried this:

      let funcRef: (Optional<Any>) -> (String) -> Any = Optional.require
      let optionalRequire: UInt64 = unsafeBitCast(funcRef, to: UInt64.self)

but it crashes with Can't unsafeBitCast between types of different sizes

[edenman]

@kattrali any idea about the above?