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

Feature: Capture console.log instead of using separate log function #1

Open
IjzerenHein opened this issue Jul 20, 2016 · 7 comments
Open

Comments

@IjzerenHein
Copy link

Hi, I think it would be nicer if all console.log calls are automatically logged for crash reporting (instead of having to call FirebaseCrash.log).

Like this (Objective-C example):

  //Add the following lines
  RCTSetLogThreshold(RCTLogLevelInfo);
  RCTSetLogFunction(FIRReactLogFunction);
RCTLogFunction FIRReactLogFunction = ^(
                                               RCTLogLevel level,
                                               __unused RCTLogSource source,
                                               NSString *fileName,
                                               NSNumber *lineNumber,
                                               NSString *message
                                               )
{
  NSString *log = RCTFormatLog([NSDate date], level, fileName, lineNumber, message);

#ifdef DEBUG
  fprintf(stderr, "%s\n", log.UTF8String);
  fflush(stderr);
#else
  FIRCrashLog(log);
#endif

  int aslLevel;
  switch(level) {
    case RCTLogLevelTrace:
      aslLevel = ASL_LEVEL_DEBUG;
      break;
    case RCTLogLevelInfo:
      aslLevel = ASL_LEVEL_NOTICE;
      break;
    case RCTLogLevelWarning:
      aslLevel = ASL_LEVEL_WARNING;
      break;
    case RCTLogLevelError:
      aslLevel = ASL_LEVEL_ERR;
      break;
    case RCTLogLevelFatal:
      aslLevel = ASL_LEVEL_CRIT;
      break;
  }
  asl_log(NULL, NULL, aslLevel, "%s", message.UTF8String);
};

It is described in more detail here:
https://medium.com/delivery-com-engineering/add-crashlytics-to-your-react-native-ios-app-69a983a9062a#.rg68fvfs7

@ianlin
Copy link
Owner

ianlin commented Jul 21, 2016

Hi @IjzerenHein, thanks for noticing this. We are using a logging abstraction layer in Javascript with this module, e.g.

import FirebaseCrash from 'react-native-firebase-crash-report';

export default class Log {
    constructor(props) {
        this.logToConsole = props.logToConsole;
        this.logToFirebase = props.logToFirebase;
    }

    log(msg) {
        if (this.logToConsole) {
            console.log(msg);
        }
        if (this.logToFirebase) {
            FirebaseCrash.log(msg);
        }
    }

However I think you are right, making it in native side is much better for performance. But it might not be a feature to this module IMO since it can be all done through editing AppDelegate.m without React Native's bridge. Let me know if it make sense to you or not, cheers!

@IjzerenHein
Copy link
Author

Hi @ianlin. Yes you're true, you could integrate it in AppDelegate (that's exactly what I've done so far 🙃). Yet, I'd still love to see this in the form of an installable plugin rather than custom AppDelegate code.

So, here is my case for using automatically hooking into console.log:

  • Console.log support n+1 arguments so you can do things like this console.log("hello", "world: ", {a: 1, b: 2})
  • Standard way to support log-levels using console.info, console.warn, console.error
  • Warnings throws by react-native are automatically logged (they use console.warn)
  • Code portability, when you use console.log, the code can be used on the web as well, and could easily switch to another crash reporting tool if you want
  • Log messages generated by other plugins are also captured

Also, I think it would be really nice if this is a feature of this plugin. Perhaps one that you could enable conditionally, but in my perception it should be enabled always when you choose to use this plugin. Also, most people don't know that you can intercept react-native's log messages. Having this code in a central plugin that everyone uses makes it much easier to maintain for all of us.

Cheers :)

@ianlin
Copy link
Owner

ianlin commented Jul 24, 2016

That sounds great, will try to make it soon. Btw do you have any idea of overriding react-native's logging functions in Android?

@IjzerenHein
Copy link
Author

Hi! So for Android I currently only capture console.log, console.info and console.error' Any log-messages that are logged in the native-code are currently nog logged.

I'm using Javascript code to intercept console.log:

function init() {
    if (Platform.OS === 'android') {
        console.log = interceptLog(console.log);
        console.info = interceptLog(console.info);
        console.error = interceptLog(console.error);
        //console.debug = interceptLog(console.debug);
    }
    if (!__DEV__) {
        global.ErrorUtils.setGlobalHandler(errorHandler);
    }
}

function interceptLog(originalFn) {
    return function() {
        const args = Array.prototype.slice.apply(arguments);
        let result = '';
        for (let i = 0; i < args.length; i++) {
            const arg = args[i];
            if (!arg || (typeof arg === 'string') || (typeof arg === 'number')) {
                result += arg;
            }
            else {
                result += JSON.stringify(arg);
            }
        }
        //originalFn.call(console, 'INTERCEPTED LOG: ' + result);
        FirebaseCrash.log(result);
        return originalFn.apply(console, arguments);
    };
}

@temitope
Copy link

temitope commented Apr 6, 2017

@ianlin @IjzerenHein thanks for the code and the discussion, super helpful.

Do any of you see any reason why the interceptLog code @IjzerenHein shared shouldn't (or couldn't) be used for iOS? I noticed that it was only done for android in the code. If possible I would love to avoid modifying the AppDelegate.m file entirely and simply do this in pure JS on the react-native side for both platforms.

Cheers

@IjzerenHein
Copy link
Author

@temitope No, there's no reason why that code shouldn't work on iOS too. When I pasted the example, I happened to have a clause for Android only in there at that moment, no idea why anymore. I've been using it successfully for iOS as well.

@temitope
Copy link

temitope commented Apr 6, 2017

@IjzerenHein great news. thanks!

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

3 participants