Segment containing BigInt in metadata throws an error when serialized

[dreamorosi]

Hi, one of our customers (Powertools for AWS (TypeScript)) reported (aws-powertools/powertools-lambda-typescript#1713) that the addMetadata method throws an error when the metadata object contains a BigInt value.

Example:

const segment = new Subsegment('## foo.bar');

segment.addMetadata('foo', {
  bar: BigInt(123),
});

BigInt is a native JavaScript object but cannot be serialized by the built-in JSON.stringify() function. Because of this, the SDK allows you to add the value as metadata but eventually throws an error when the segment is serialized right before being emitted.

The JSON.stringify() object allows a replacer function as second argument, which could be used to encode this, as well as other exotic objects.

In this case, the change would be minimal and be applied to the format() and toString() methods of the Segment object, since they both use call JSON.stringify() here and here.

After the change, the implementation would look something like this:

Subsegment.prototype.format = function format() {
    this.type = 'subsegment';
    if (this.parent) {
        this.parent_id = this.parent.id;
    }
    if (this.segment) {
        this.trace_id = this.segment.trace_id;
    }
--  return JSON.stringify(this);
++ return JSON.stringify(this, replacer());
}; 
/**
 * Returns the formatted subsegment JSON string.
 */
Subsegment.prototype.toString = function toString() {
--  return JSON.stringify(this);
++ return JSON.stringify(this, replacer());
};

++ function replacer(_, value) {
++   if (typeof value === 'bigint') {
++     return value.toString();
++   }
++
++   return value;
++ }

As mentioned, the change would be minimal and would impact only BigInt objects, for all other types the added code would be equivalent to a no-op.

As an alternative solution, if the replacer idea is not welcome, I would propose to extract the JSON.stringify() calls to its own method in the Segment object so that it can be easily overridden/extended by consumers, i.e. :

Subsegment.prototype.format = function format() {
    this.type = 'subsegment';
    if (this.parent) {
        this.parent_id = this.parent.id;
    }
    if (this.segment) {
        this.trace_id = this.segment.trace_id;
    }
--  return JSON.stringify(this);
++ return this.serialize();
}; 
/**
 * Returns the formatted subsegment JSON string.
 */
Subsegment.prototype.toString = function toString() {
--  return JSON.stringify(this);
++ return this.serialize();
};

++ Subsegment.prototype.serialize = function serialize() {
++   return JSON.stringify(this);
++ };

This second option would have a smaller footprint and would still help us (Powertools) fix the issue, however I'd prefer the previous option so that all X-Ray customers can avoid seeing errors.

We would love to contribute the change directly back to the SDK so that it can benefit all customers and not just Powertools ones. If I were to open a PR, would be open to review it and help with the merge+release process once it's up to the project's standards?

[wangzlei]

Thanks for raising this issue. May I ask in what scenario customer may want to use BigInt instead of String or Int?

[dreamorosi]

Hi @wangzlei, thanks for replying to the issue.

This specific customer is adding the content of a response from an upstream service directly as metadata, you can read the example in their words in the issue I linked above.

Either way, BigInt is a native JavaScript object and the current behavior of throwing an unhandled exception should not be acceptable since it results in a guaranteed data loss of the entire segment and potential disruption of the service being traced.