Skip to content

attributeEncoders have never been registered (all v3 releases), and the unsupported-value drop path throws on prototype-less objects #1426

Description

@harrisrobin

Describe the bug

Two related defects in the attribute-encoding pipeline. Together they mean that attributeEncoders has never had any effect in a published v3 release, and that the "drop unsupported values instead of crashing" guarantee in MIGRATION.md does not hold for prototype-less objects.

1. Configured attributeEncoders are never registered.

DdSdkWrapper.initialize is the only non-test code that populates the encoder registry:

// packages/core/src/sdk/DdSdkInternal.ts:32
initialize(configuration: DdSdkNativeConfiguration): Promise<void> {
    this._attributeEncoders = [...configuration.attributeEncoders];
    return NativeDdSdk.initialize(configuration);
}

It has no caller. Every initialization path funnels through DdSdkReactNative.initializeNativeSDK, which bypasses the wrapper and calls the native module directly:

// packages/core/src/DdSdkReactNative.tsx:100
await NativeDdSdk.initialize(
    DdSdkReactNative.buildConfiguration(configuration, params)
);

So DdSdk.attributeEncoders — the list read by encodeAttributes (AttributesEncoding/attributesEncoding.ts) and by arrayEncoder / errorEncoder / mapEncoder (AttributesEncoding/defaultEncoders.ts) — stays [] for the process lifetime, and every consumer-supplied encoder is dead code.

This appears to date back to the commit that introduced the feature, d2b1793 ("Attributes Safe Encoding"). That commit renamed the raw native module from DdSdk to NativeDdSdk and rebound the name DdSdk to the new wrapper. The rename was applied to every DdSdk.* call site in DdSdkReactNative.tsx — including the initialize call, which was the one that needed to keep pointing at the wrapper:

-import { DdSdk } from './sdk/DdSdk';
+import { NativeDdSdk } from './sdk/DdSdkInternal';
@@
-        await DdSdk.initialize(
+        await NativeDdSdk.initialize(
             DdSdkReactNative.buildConfiguration(configuration, params)
         );

I checked the published tarballs for 3.0.0 (first v3 release) through 3.7.0 and the current develop: all of them call NativeDdSdk.initialize there, and sdk/AttributesEncoding/ is byte-identical between 3.5.3 and 3.7.0.

The existing test coverage would not catch this — AttributesEncoding/__tests__/attributesEncoding.test.ts:17 injects encoders through DdSdk._setAttributeEncodersForTesting(), so the real registration path is never exercised.

2. The "unsupported value" drop path throws on prototype-less objects.

MIGRATION.md states that unsupported values "are dropped with a warning to prevent crashes and undefined behavior". The drop itself calls String(value) unguarded:

// packages/core/src/sdk/AttributesEncoding/helpers.ts:97
`Dropped unsupported value at '${formatPathForLog(path)}': ${String(value)}`

(and the same pattern at helpers.ts:72 for array items).

An object created with Object.create(null) is not matched by isPlainObject (which tests v.constructor === Object) and is not matched by any built-in encoder, so it reaches this branch. String() on it throws — under Hermes, TypeError: Cannot determine default value of object. Because encodeAttributes is called inside bufferVoidNativeCall, the throw surfaces as an unhandled promise rejection and the RUM action or error is silently lost.

The two defects mask each other: a consumer encoder is the documented escape hatch for defect 2, but defect 1 means it can never fire.

Concretely, this reaches us via Expo Router, which builds route params with Object.create(null) (expo-router/build/global-state/getRouteInfoFromState.js:39, build/fork/getStateFromPath.js:284, and two other sites). Those params end up in RUM action attributes, so every tracked tab press throws.

Reproduction steps

import {
    CoreConfiguration,
    DdSdk,
    DdSdkReactNative,
    DdRum,
    RumActionType,
    TrackingConsent,
} from '@datadog/mobile-react-native';

const config = new CoreConfiguration(
    '<client-token>',
    'test',
    TrackingConsent.GRANTED,
    {
        rumConfiguration: { applicationId: '<app-id>' },
        attributeEncoders: [
            {
                check: (v: unknown) =>
                    typeof v === 'object' &&
                    v !== null &&
                    Object.getPrototypeOf(v) === null,
                encode: (v: object) => Object.fromEntries(Object.entries(v)),
            },
        ],
    }
);

await DdSdkReactNative.initialize(config);

// Defect 1: the configured encoder was never registered.
console.log(DdSdk.attributeEncoders); // []  — expected the encoder above

// Defect 2: with no encoder able to claim it, this rejects instead of
// dropping the value.
const params = Object.create(null);
params.query = 'Lisbon';

await DdRum.addAction(RumActionType.TAP, 'search', { params });
// TypeError: Cannot determine default value of object

The same happens through DdRum.addError / DdRumErrorTracking, so error reporting is affected too.

SDK logs

Uncaught (in promise) TypeError: Cannot determine default value of object
  encodeAttributesInPlace   AttributesEncoding/helpers.ts:97
  encodeAttributes          AttributesEncoding/attributesEncoding.ts:37
  bufferVoidNativeCall      DdRum.ts:243
  DdRumWrapper#addAction    DdRum.ts:238

Expected behavior

  1. attributeEncoders supplied on CoreConfiguration should be registered on DdSdk during initialization, for every init path (DdSdkReactNative.initialize, DatadogProvider, and the deferred/partial variants). Routing initializeNativeSDK through DdSdk.initialize rather than NativeDdSdk.initialize looks like it would cover all of them in one place — buildConfiguration always returns a CoreConfiguration-derived object whose attributeEncoders defaults to [], so the spread is safe on every path.

  2. An unsupported value should be dropped with a warning, as documented, rather than throwing — the String(value) calls at helpers.ts:72 and helpers.ts:97 should be made exception-safe (e.g. a try/catch falling back to Object.prototype.toString.call(value)).

  3. It would be worth covering the registration path in a test that does not go through _setAttributeEncodersForTesting, since that seam is what allowed defect 1 to ship.

Affected SDK versions

3.0.0 – 3.7.0 (all v3 releases), and current develop

Latest working SDK version

None — attributeEncoders was introduced in 3.0.0 and has never been registered. v2 is unaffected because it has no such option.

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

Yes — reproduced against 3.7.0.

Integration Methods

NPM

React Native Version

0.86.2

Package.json Contents

{
  "@datadog/mobile-react-native": "3.5.3",
  "@datadog/mobile-react-navigation": "3.5.3",
  "expo": "57.0.13",
  "expo-datadog": "57.0.0",
  "expo-router": "~57.0.13",
  "react-native": "0.86.2"
}

Other relevant information

Both defects are pure JS, independent of platform and of native setup. Verified by reading the published npm tarballs for 3.0.0, 3.5.3 and 3.7.0, plus develop.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions