Error property in log and RUM context is stripped of properties

[max-nicholson]

Describe the bug

DdLogs.[level] and DdRum.addAction strips properties from context.error

Using latest minor version of "@datadog/mobile-react-native", through "expo-datadog"

On Android

With a basic error

const error = new Error("foo")

When they reach the Log Explorer, these will have no @error.* attributes whatsoever

DdLogs.info('message', { error })
DdLogs.info('message', { error: { message: error.message, stack: error.stack, name: error.name })

When they reach the Log Explorer, these will have an @error.message, @error.stack and @error.kind, but no other Error properties

DdLogs.info('message', error.name, error.message, error.stack, { error })

Error.cause is also stripped, as are any custom properties if using a custom Error (the below will have no @error.* attributes in Log Explorer)

class MyCustomError extends Error {
  name = 'MyCustomError'

  foo = 'bar'
}

DdLogs.info('message', { error: new MyCustomError("foo", { cause: "bar" }) })

On iOS

We get a little more than on Android, but still not much

Most we only get:

{
  "error": {
    "jsEngine": "hermes"
  }

For a custom error class, then we see slightly more e.g. an Urql Network error (https://github.com/urql-graphql/urql) appears in Log Explorer like

{
  "error": {
    "jsEngine": "hermes"
    "name": "CombinedError"
     "networkError": {
        "jsEngine": "hermes"
      }
    }
}

Notes

The only workaround I found to ensure all "error" properties were kept was the following:

DdLogs.info('message', error.name, error.message, error.stack, {
  otherNonErrorContextProperty: 'value',
  'error.name': error.name,
  'error.customProperty': error.customProperty,
  'error.cause': {
    'message': error.cause.message,
    'stack': error.cause.stack,
    'name': error.cause.name,
    'customProperty': error.cause.customProperty,
  },
})

but this feels like this should be a responsibility of the library, not the caller

Reproduction steps

The following test passes + the differing behaviour between Android and iOS suggests it's in the native module side rather than this SDK

import { DdLogs } from '@datadog/mobile-react-native'
import { NativeModules } from 'react-native'

jest.mock('react-native', () => {
  const RN = jest.requireActual('react-native')

  RN.NativeModules.DdLogs = {
    info: jest.fn().mockImplementation(
      () =>
        new Promise<void>(resolve => {
          resolve()
        })
    ),
  }

  return RN
})

describe('tracking', () => {
  beforeEach(() => {
    DdLogs.unregisterLogEventMapper()
  })

  it('test', async () => {
    class CustomError extends Error {
      name = 'CustomError'

      foo = 'bar'
    }
    const context = { error: new CustomError('foo') }

    await DdLogs.info('message', context)

    const args = NativeModules.DdLogs.info.mock.calls[0]

    expect(args[1].error.message).toBe('foo')
    expect(args[1].error.stack).toEqual(context.error.stack)
    expect(args[1].error.name).toBe('CustomError')
    expect(args[1].error.foo).toBe('bar')
  })
})

SDK logs

No response

Expected behavior

DdLogs.[level] and DdRum.addAction should preserve all (built-in and custom) properties on an error property on the context argument

Affected SDK versions

2.4.3

Latest working SDK version

N/A

Did you confirm if the latest SDK version fixes the bug?

Yes

Integration Methods

Yarn

React Native Version

0.73.6

Package.json Contents

{
"dependencies": {
"expo-datadog": "51.0.2",
"react-native": "0.73.6",
"@datadog/mobile-react-native": "2.4.3"
}
}

iOS Setup

No response

Android Setup

No response

Device Information

No response

Other relevant information

No response

[marco-saia-datadog]

Hi @max-nicholson! 👋 thank you for reaching out :)

As a reference I will paste the arguments that all the DdLogs "with errors" APIs accept:

export type LogWithErrorArguments = [
    message: string,
    errorKind?: string,
    errorMessage?: string,
    stacktrace?: string,
    context?: object,
    fingerprint?: string,
    source?: ErrorSource
];

And its simplified overload:

export type LogArguments = [message: string, context?: object];

In your case, you want to keep track of all the error properties, by passing it inside the context.

I believe the problem is that, as the context has to be serializable in order to reach the native layer, it might be received as an empty object because Error in JavaScript does not have any enumerable properties.

What can you do?

If that's the root of the issue, you could write your serialize function once and use it on the errors that you want to report, as you mentioned in your description

e.g

function serializeError(error: Error) {
  return {
    'error.name': error.name,
    'error.customProperty': error.customProperty,
    'error.cause': {
      'message': error.cause.message,
      'stack': error.cause.stack,
      'name': error.cause.name,
      'customProperty': error.cause.customProperty,
  },
}

This would work because you are retrieving the values of the non-enumerable properties of the error, and you are manually inserting them into your context object.

What can we do?

1. Improve our validation of the context: it's our responsibility to report whether the context is valid or not
2. Provide a standardized way to serialise the error object: we could improve our SDK so that if an error object is passed in the context, we perform a default serialization on it to make sure no properties are lost

I'd be glad if you could confirm that this is indeed the problem and in the meantime we will plan some improvements, and keep you posted on the topic and on future evolutions of our SDK.

Does it sound good to you? :)

[max-nicholson]

Hi @marco-saia-datadog, yes that's correct.

The main gotcha with the current behaviour is that you need nested key dotpaths (e.g. "error.name", "error.customProperty") vs. simply converting the Error instance to a plain object. This behaviour isn't the case for any other key on the context object e.g. a context of

{
  "foo:" {
    "bar": "baz"
  }
}

works, but

{
  "error": {
     "message": "foo",
     "bar": "baz"
  }
}

does not.

This also doesn't match the behaviour of @datadog/browser-logs (where serializing an Error instance to a plain object before logging it works fine)

Either your 2. proposal, or aligning the behaviour to the browser-logs i.e. allowing

{
  "error": {
     "message": "foo",
     "bar": "baz"
  }
}

would make sense to me.