Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Every error is rb_vm_raise Is this normal? #3

Open
Bodacious opened this issue May 3, 2016 · 19 comments
Open

Every error is rb_vm_raise Is this normal? #3

Bodacious opened this issue May 3, 2016 · 19 comments

Comments

@Bodacious
Copy link

Hi guys

I've installed motion-fabric to use crashlytics.

It seems to be catching the errors OK, but they are all listed as rb_vm_raise errors, under the same issue.

Is this the expected behaviour, or am I doing something dumb?

@MarkVillacampa
Copy link
Member

Crashlytics groups the crash reports by the name of the last symbol in the stack trace.

In this case, rb_vm_raise is the internal RubyMotion function where exceptions are raised, hence it is a common place where applications would crash.

It would be ideal if there was an option to group crashes by exception message or other metric.

I'm leaving this issue open and will update it if I find a way to do this.

@Bodacious
Copy link
Author

Bodacious commented May 26, 2016

It would be great if something could be done on this.

Our clients have access to the Crashlyrics account too, and they have no way of knowing what these messages mean so assume everything is urgent. >.<

@mattmassicotte
Copy link

Hey All - Matt from Crashlytics here.

This is "normal" in the sense that Crashlytics' backend system needs fairly extensive knowledge about system internals to do even semi-intelligent crash grouping. We have lots of custom rules/patterns for iOS itself, but not for ruby motion's internals. We only group on the last symbol as a last resort when we have no other means of determining what to blame.

Now, there is some hope :)

We have an API (iOS only I'm afraid) that is specifically built to accommodate this kind of environment. See -recordCustomExceptionName:reason:frameArray: in Crashlytics.h. It could allow you to actually expose a pure-Ruby stack trace in our UI. Now, this still would not solve the problem of our server-side analysis not understanding your code/environment. But, it might prove to be a better approach. I've seen it work well enough for Lua, Javascript, and C# in production.

Nothing is going to be as good as us making a custom analysis engine, but that's not something we can commit to at the moment. For now, I hope this is a) technically feasible b) enough to get you by for now.

@MarkVillacampa
Copy link
Member

@mattmassicotte Thanks for chiming in!

I will investigate in this direction and try to build a system to report more friendly stack traces 🙂

@mattmassicotte
Copy link

I'm happy to help however I can. Let me know how this goes!

@MarkVillacampa
Copy link
Member

MarkVillacampa commented Jun 3, 2016

@mattmassicotte Im making some progress, but just got stuck in something:

  • I set a custom exception handler with std::set_terminate (after Crashlytics has set their's), process the exception, and log it to Crashlytics using recordCustomExceptionName:reason:frameArray:
  • The exception will be logged twice, once with my custom processed exception, and the other via Crashlytics default handling.

I thought about NOT calling Crashlytics exception handler from my own exception handler, but then if I call abort() to terminate the process, Crashlytics signal handler will log that as well.

Any idea how to prevent Crashlytics from logging the default exception and prevent the signal handler from logging an exception I have already handled?

Also, do you know of any open source implementation of something similar to this? Maybe one of those you mentioned (Lua, Javascript, and C#)

Thanks!

@mattmassicotte
Copy link

I'm not aware of an open-source example, unfortunately.

Can you explain to me why you need the set_terminate? Does RubyMotion rely on the C++ exception mechanism for Ruby exceptions? If so, this will be trickier. I don't believe this is how Lua works, and is definitely not how Javascript works either. Both propagate the exception within their own VMs, and have an opportunity to call native code before terminating the process.

@MarkVillacampa
Copy link
Member

MarkVillacampa commented Jun 3, 2016

RubyMotion compiles .rb files into LLVM IR that is then linked into the final executable. Exceptions are really Objc exceptions and use the C++ exception mechanism. For all Crashlytics knows, RubyMotion exceptions are Objc exceptions.

The issue comes from the fact that the stack trace contains symbols from methods of the application AND internal symbols from the RubyMotion VM. This is better understood with an example:

Fatal Exception: NameError
0  sample-app                     0x1002a1093 rb_vm_raise
1  sample-app                     0x1001983d9 rb_exc_raise
2  sample-app                     0x10029f3fd rb_vm_method_missing
3  sample-app                     0x100271115 rb_vm_dispatch
4  sample-app                     0x10026fe7c rb_vm_trigger_method_missing
5  sample-app                     0x100271421 rb_vm_dispatch
6  sample-app                     0x10015161c vm_dispatch
7  sample-app                     0x100151c1a rb_scope__foo__ (app_delegate.rb:9)
8  sample-app                     0x100271b35 rb_vm_dispatch
9  sample-app                     0x100151b0d rb_scope__lol__ (app_delegate.rb:4)
10 sample-app                     0x100271b35 rb_vm_dispatch
11 sample-app                     0x100153467 rb_scope__application:didFinishLaunchingWithOptions:__ (app_delegate.rb:58)
12 sample-app                     0x100153524 __unnamed_16
13 UIKit                          0x100f629ac (Missing)
14 UIKit                          0x100f63c0d (Missing)
15 UIKit                          0x100f6a568 (Missing)
16 UIKit                          0x100f67714 (Missing)
17 FrontBoardServices             0x1065568c8 (Missing)
18 FrontBoardServices             0x106556741 (Missing)
19 FrontBoardServices             0x106556aca (Missing)
20 CoreFoundation                 0x103092301 (Missing)
21 CoreFoundation                 0x10308822c (Missing)
22 CoreFoundation                 0x1030876e3 (Missing)
23 CoreFoundation                 0x1030870f8 (Missing)
24 UIKit                          0x100f66f21 (Missing)
25 UIKit                          0x100f6bf09 (Missing)
26 sample-app                     0x1001505bd main (main.mm:16)
27 libdyld.dylib                  0x10604192d (Missing)

For all the symbols from the application binary (sample-app), only those that are symbolicated correspond to Ruby methods written by RubyMotion users:

7  sample-app                     0x100151c1a rb_scope__foo__ (app_delegate.rb:9)
9  sample-app                     0x100151b0d rb_scope__lol__ (app_delegate.rb:4)
11 sample-app                     0x100153467 rb_scope__application:didFinishLaunchingWithOptions:__ (app_delegate.rb:58)

In this case rb_scope__foo__ (app_delegate.rb:9) corresponds to:

class AppDelegate
   def foo
   end
end

The rest of the symbols, are internal symbols from the RubyMotion VM:

0  sample-app                     0x1002a1093 rb_vm_raise
1  sample-app                     0x1001983d9 rb_exc_raise
2  sample-app                     0x10029f3fd rb_vm_method_missing
3  sample-app                     0x100271115 rb_vm_dispatch
4  sample-app                     0x10026fe7c rb_vm_trigger_method_missing
5  sample-app                     0x100271421 rb_vm_dispatch
6  sample-app                     0x10015161c vm_dispatch
8  sample-app                     0x100271b35 rb_vm_dispatch
10 sample-app                     0x100271b35 rb_vm_dispatch

These symbols not only give little useful information to the user, but also confuse Crashlytics grouping mechanism into believing all crashes whose last symbol from the application binary is e.g. rb_vm_raise, are really the same issue.

My idea is to use a custom exception handler which takes only the relevant symbols from callStackReturnAddresses array, and logs the crash with Crashlytics.

@MarkVillacampa
Copy link
Member

I've noticed a couple differences when using recordCustomExceptionName:reason:frameArray versus the normal crash logging:

  • Stack traces from all threads are not recorded
  • Crashes are marked as Non-fatal

This is the same exception, with custom logging and with default logging

@mattmassicotte
Copy link

Ah - yes. Marking them as non-fatal was, I believe, a bug. For reasons, its not straight-forward for us to fix it. The lack of other stack traces is kinda surprising, but I haven't looked into it recently. Once you do the custom logging, does the process exit?

@MarkVillacampa
Copy link
Member

This is the code I have come up with:

#import "Crashlytics/Crashlytics.h"
#import <exception>
#import <dlfcn.h>

OBJC_EXTERN void *__cxa_allocate_exception(size_t thrown_size);
OBJC_EXTERN void __cxa_throw(void *exc, void *typeinfo, void (*destructor)(void *)) __attribute__((noreturn));
OBJC_EXTERN void *__cxa_begin_catch(void *exc);
OBJC_EXTERN void __cxa_end_catch(void);
OBJC_EXTERN void __cxa_rethrow(void);
OBJC_EXTERN void *__cxa_current_exception_type(void);

static void (*old_terminate)(void) = nil;

#define BLACKLIST_SIZE 6
static char *blacklist[BLACKLIST_SIZE] = {
    "rb_vm_raise",
    "rb_exc_raise",
    "rb_vm_dispatch",
    "vm_dispatch",
    "rb_vm_trigger_method_missing",
    "rb_vm_method_missing"
};

void
SendExceptionToCrashlytics(NSException*exception)
{
    NSArray *addresses = [exception callStackReturnAddresses];
    NSMutableArray *stack_frames = [NSMutableArray new];

    for (NSNumber *address in addresses) {
    Dl_info info;
    if (dladdr((void*)[address unsignedIntegerValue], &info) == 0) {
        continue;
    }

    bool should_continue = false;
    for (int i = 0; i < BLACKLIST_SIZE; i++) {
        if (strcmp(blacklist[i], info.dli_sname) == 0) {
        should_continue = true;
        break;
        }
    }

    if (should_continue) {
        continue;
    }

    CLSStackFrame *stack_frame = [CLSStackFrame stackFrameWithAddress:[address unsignedIntegerValue]];
    [stack_frames addObject:stack_frame];
    }
    [[Crashlytics sharedInstance] recordCustomExceptionName:[exception name] reason:[exception reason] frameArray:stack_frames];
}

void
RubyMotionCrashlyticsExceptionHandler(void)
{
    if (! __cxa_current_exception_type()) {
        // No current exception.
        (*old_terminate)();
    }
    else {
        // There is a current exception. Check if it's an objc exception.
        @try {
            __cxa_rethrow();
        } @catch (id e) {
        SendExceptionToCrashlytics((NSException*)e);
        (*old_terminate)();
        } @catch (...) {
            // It's not an objc object. Continue to C++ terminate.
            (*old_terminate)();
        }
    }
}

extern "C" 
void 
RubyMotionCrashlyticsInstallExceptionHandler(void)
{
    old_terminate = std::set_terminate(&RubyMotionCrashlyticsExceptionHandler);
}

So right now the I do not exit after the custom logging, I call the next exception handler.

As you can see, the symbol blacklist is pretty small at this point. I was wondering, what are the current heuristics Crashlytics uses to determine which frames are blamed for a crash? Would it be possible for us (RubyMotion) to provide a list of internal symbols to mark as never to be blamed for a crash?

@mattmassicotte
Copy link

I think that would be the simplest option, and would produce the best results. It does require some work on our end, which I cannot commit to at the moment. Let me get back to you, post-WWDC.

@MarkVillacampa
Copy link
Member

That would be awesome! :)

I've implemented this workaround in a production app so I can start gathering information on which other symbols we may need to filter out.

@mattmassicotte
Copy link

After speaking with the team, I don't think this is something that we can address in the immediate term on our end. We've noted it needs to happen, and will keep you up to date on any changes here.

Please let me know how your experiment goes as well.

@andrewhavens
Copy link

@mattmassicotte @MarkVillacampa I'm wondering if any progress was made on this since the last comment. We're running into this issue as well and it makes it hard to filter through the crashes when they are all grouped together.

@mattmassicotte
Copy link

@andrewhavens I'm afraid I no longer work on the Fabric/Crashlytics products. I don't have access to the code anymore - but I'm happy to consult and/or refer as I'm able. In the meantime, I'd definitely recommend reaching out to the Crashlytics support people.

@MarkVillacampa
Copy link
Member

MarkVillacampa commented Aug 15, 2018

@andrewhavens have you tried my workaround here? It worked fairly well for me in production.

EDIT: this is an updated version with some extra function names added

#import "Crashlytics/Crashlytics.h"
#import <exception>
#import <dlfcn.h>

OBJC_EXTERN void *__cxa_allocate_exception(size_t thrown_size);
OBJC_EXTERN void __cxa_throw(void *exc, void *typeinfo, void (*destructor)(void *)) __attribute__((noreturn));
OBJC_EXTERN void *__cxa_begin_catch(void *exc);
OBJC_EXTERN void __cxa_end_catch(void);
OBJC_EXTERN void __cxa_rethrow(void);
OBJC_EXTERN void *__cxa_current_exception_type(void);

static void (*old_terminate)(void) = nil;

#define BLACKLIST_SIZE 16
static const char *blacklist[BLACKLIST_SIZE] = {
    "rary_aref",
    "rb_const_get_0",
    "rb_exc_raise",
    "rb_exc_new",
    "rb_invalid_str",
    "rb_mod_const_missing",
    "rb_vm_raise",
    "rb_vm_dispatch",
    "rb_vm_trigger_method_missing",
    "rb_vm_method_missing",
    "send_internal",
    "rb_f_raise",
    "rb_Integer",
    "rb_num2long",
    "send_internal",
    "vm_dispatch"
};

extern "C" 
void
SendExceptionToCrashlytics(NSException*exception)
{
    NSArray *addresses = [exception callStackReturnAddresses];
    NSMutableArray *stack_frames = [NSMutableArray new];
    
    for (NSNumber *address in addresses) {
	Dl_info info;
	if (dladdr((void*)[address unsignedIntegerValue], &info) == 0) {
	    continue;
	}

	bool should_continue = false;
	for (int i = 0; i < BLACKLIST_SIZE; i++) {
	    if (strcmp(blacklist[i], info.dli_sname) == 0) {
		should_continue = true;
		break;
	    }
	}

	if (should_continue) {
	    continue;
	}

	CLSStackFrame *stack_frame = [CLSStackFrame stackFrameWithAddress:[address unsignedIntegerValue]];
	[stack_frames addObject:stack_frame];
    }
    [[Crashlytics sharedInstance] recordCustomExceptionName:[exception name] reason:[exception reason] frameArray:stack_frames];
}

void
RubyMotionCrashlyticsExceptionHandler(void)
{
    if (! __cxa_current_exception_type()) {
        // No current exception.
        (*old_terminate)();
    }
    else {
        // There is a current exception. Check if it's an objc exception.
        @try {
            __cxa_rethrow();
        } @catch (id e) {
	    SendExceptionToCrashlytics((NSException*)e);
	    (*old_terminate)();
        } @catch (...) {
            // It's not an objc object. Continue to C++ terminate.
            (*old_terminate)();
        }
    }
}

extern "C" 
void 
RubyMotionCrashlyticsInstallExceptionHandler(void)
{
    old_terminate = std::set_terminate(&RubyMotionCrashlyticsExceptionHandler);
}

@andrewhavens
Copy link

Hey @MarkVillacampa Thanks for chiming in. I thought you said there were still problems with this approach. Didn't you say the exception would be logged twice? Also, can you tell me how I would integrate this patch into my app? 😄 Can we update the motion-fabric gem to automatically apply this patch?

@MarkVillacampa
Copy link
Member

MarkVillacampa commented Aug 15, 2018

The exception would be recorded twice, once as a crash, and once as a non-fatal exception:

screenshot 2018-08-15 at 17 25 35

You'll need to ignore the crashes whose last function in the stack trace is included in the blacklist.

The way you'd integrate this into your application is the following:

  • Add this piece of code early in the lifecycle of your app. The order is important:
      Fabric.with([Crashlytics.sharedInstance])
      RubyMotionCrashlyticsInstallExceptionHandler()
  • Create a directory inside the vendor directory called RubyMotionExceptionHandler and add a .mm file containing the above code, and a .h header containing the following:
void RubyMotionCrashlyticsInstallExceptionHandler(void);
  • Add the following to your Rakefile:
  app.vendor_project("vendor/trace-helper", :static,
     cflags: "-F'#{File.expand_path(Motion::Project::CocoaPods::PODS_ROOT)}/Crashlytics/iOS/'")

And that's it! It can definitely be included in motion-fabric.

Let me know if it works.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants