DataDog / datadog-lambda-js

The Datadog AWS Lambda Library for Node
Apache License 2.0
105 stars 35 forks source link

Circular references are not handled in tag-object when trying to serialize Lambda's response #541

Closed rdsedmundo closed 2 months ago

rdsedmundo commented 2 months ago

Expected Behavior

Circular references are handled.

Actual Behavior

Circular references are not being handled, which is a breaking change because this used to work fine.

JSON.stringify was added in https://github.com/DataDog/datadog-lambda-js/pull/430/files#diff-2c7cc9dfba401c37307fa3e04f3a3d5890bb3e8b8537c15e3cd6c5a7df72e1f1R5.

Steps to Reproduce the Problem

  1. Enable Lambda request/response payload tracking.
  2. Return a response with a circular object.
  3. Observe that an error is thrown, e.g. with a Sequelize object being returned (see stacktrace below).

Specifications

Stacktrace

{"errorType":"TypeError","errorMessage":"Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'include' -> object with constructor 'Array'
    |     index 0 -> object with constructor 'Object'
    --- property 'parent' closes the circle","stack":["TypeError: Converting circular structure to JSON","    --> starting at object with constructor 'Object'","    |     property 'include' -> object with constructor 'Array'","    |     index 0 -> object with constructor 'Object'","    --- property 'parent' closes the circle","    at JSON.stringify (<anonymous>)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:41:35)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at tagObject (/opt/nodejs/node_modules/datadog-lambda-js/utils/tag-object.js:65:17)","    at TraceListener.onEndingInvocation (/opt/nodejs/node_modules/datadog-lambda-js/trace/listener.js:148:35)","    at /opt/nodejs/node_modules/datadog-lambda-js/index.js:235:80","    at step (/opt/nodejs/node_modules/datadog-lambda-js/index.js:44:23)","    at Object.next (/opt/nodejs/node_modules/datadog-lambda-js/index.js:25:53)"]}

@joeyzhao2018

purple4reina commented 2 months ago

Hey @rdsedmundo, thanks for reporting this. I'm having issues recreating though. You say that "this used to work fine". However, I'm seeing that even without this datadog library installed, lambda is still throwing an error:

2024-05-14T16:52:30.808Z 9a6a6ede-7578-4524-b42f-80dc66f8aa58 ERROR Invoke Error {"errorType":"Error","errorMessage":"Unable to stringify response body","stack":["Error: Unable to stringify response body"," at _trySerializeResponse (file:///var/runtime/index.mjs:527:15)"," at RAPIDClient.postInvocationResponse (file:///var/runtime/index.mjs:433:26)"," at complete (file:///var/runtime/index.mjs:811:16)"," at done (file:///var/runtime/index.mjs:835:11)"," at succeed (file:///var/runtime/index.mjs:839:9)"," at file:///var/runtime/index.mjs:872:20"]}

I am using this handler:

exports.handle = async function(event, context) {
  thing = ['a', 'b', 'c'];
  thing.push(thing);
  return thing;
}

Can you provide a code sample that demonstrates the behavior you're describing?

rdsedmundo commented 2 months ago

Ah, that's very interesting. TIL: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify#description:~:text=If%20the%20value,the%20replacer%20function%3A

Sequelize does implement the .toJSON() method, so I think the Lambda runtime is simply calling it via JSON.stringify in the original response and everything works; but the Datadog library is trying to iterate over the response and calling JSON.stringify for each sub-property, and not all of them have the .toJSON defined, only the main object itself.

What do you think it'd be a good solution here?