Bug: new crashlytics transport is not properly reporting errors

[DanielSRS]

When I use it, I get this warning: firebase.crashlytics().recordError(*) expects an instance of Error. Non Errors will be ignored.

Checking the implementation I noticed that props.msg is being passed down to recordError, but props.msg besides being typed as any is aways a string

try {
  if (isError) {
    props.options.CRASHLYTICS.recordError(props.msg);
  } else {
    props.options.CRASHLYTICS.log(props.msg);
  }
  return true;
} catch (error) {
  throw Error(
    `react-native-logs: crashlyticsTransport - Error on send msg to crashlytics`
  );
}

As a quick workaround I wrapped errors in an Error:

var customTransport: transportFunctionType = props => {
  const isError = props.level.text === 'error';
  const err = new Error(props.msg);

  crashlyticsTransport({ ...props, msg: isError ? err : props.msg });
};

but this is a bad alternative and customTransport ends up being reported as the source of the error.

---

Maybe a better approach would be to:

• use rawMsg
• check for any Error instances and reporting those and pass msg to CRASHLYTICS.log
• if none, wrapping the message in an error

but thats is just an suggestion and maybe not the best solution

[alessandro-bottamedi]

The transport is intentionally very simple, accepting only strings, because Crashlytics is used in production, where the stack trace would be incomprehensible anyway due to the JS bundle. JavaScript source maps would need to be sent to Crashlytics, but I’m not sure if that’s currently possible…

[DanielSRS]

Sorry, I think I didn't organized the info in this issue the best way. The important part is kinda hidden.

The problem is not the stack trace, but string only logs not being sent at all to crashlytcs.

When using this transport, I immediately got warnings from the firebase lib informing that those reports were going to be ignored because they're not Error instances.

---

I realize now that I should've investigated a little more. I started the integration with crashlytcs in my app and reported this as soon as I noticed, but collecting more data about the actual behavior would be better.

In short, what I noticed:

• This seems to be mostly a limitation of crashlytcs being incapable of reporting strings as erros
• this transport uses 2 crashlytcs methods: log and recordError.
  • recordError seems to only accept an Error instance as an argument, not strings.
• for the short period of time that I investigated this (a few hours) the logs didn't seem to have reached firebase.
• as a quick workaround I wrapped in a Error those logs that would've been passed to recordError method.
  • After I did that, the warning were gone and I could see some logs on firebase console.

---

Since these logs/reports can take some time to be processed by crashlytcs. I think the period of my observations were too short, and so, they can be wrong.

[ghost1face]

I just ran into this as well, what I did was this, the comment points out the issue that was noticed:

logger.createLogger({
   transport: [ crashlyticsTransport ],
   transportOptions: {
     CRASHLYTICS: {
        recordError(msg) {
           // firebase/crashlytics expects an instance of error here
          // however, react-native-logs expects msg to be type string
           crashlyticsModule.recordError(new Error(msg));
        }
     }
   }
})

Any thoughts on supporting the latest signature of recordError? https://github.com/invertase/react-native-firebase/blob/main/packages/crashlytics/lib/index.js#L118-L128 Though I'd imagine it would be a breaking change you may not be interested in bringing in