Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rum-core): stringify object rejected by a promise #1428

Merged
merged 1 commit into from Sep 19, 2023

Conversation

devcorpio
Copy link
Contributor

Solves: #1423

Context

Before this change, if the promise rejection reason was an object:

       const obj = {
              foo: "bar",
              rum: "Agent",
              hello: "world"
       };

       Promise.reject(obj)

The RUM agent was capturing the error this way:

Unhandled promise rejection: <object>. Additionally, none of the properties were included in the error custom properties, making it difficult for users to identify the error's context.

There was an exception to that rule. Objects containing a "message" property:

Screenshot 2023-09-18 at 17 03 29

But unfortunately, this is not something that users can always control.

Enhancement

From now onwards, if the rejection reason of a promise is an object (with no message property), then two things will happen:

  1. The RUM agent will JSON.stringify the obj so that the properties will be visible in the error message. This is similar to the native behaviour of browsers like Google Chrome.
  2. The non-standard properties will also be added to the error custom properties

You can see an example in the screenshots below:

Screenshot 2023-09-18 at 16 48 10 Screenshot 2023-09-18 at 16 49 21

Notes

  1. the behaviour of other types remains unchanged.

    • arrays -> <object>
    • functions -> <function>
  2. The "stringify" of the object will fail at least in two circumstances:

    • it contains a circular reference
    • one of the values is a BigInt

If that happens, a fallback will be applied, meaning that the error will be captured as if was being captured before this change.

@@ -50,6 +50,7 @@ function getApmBase() {
}

const apmBase = getApmBase()

Copy link
Contributor Author

@devcorpio devcorpio Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change to make sure the next release of RUM has the minor version increased.

The reason of this can be found here: #1402

Copy link
Member

@vigneshshanmugam vigneshshanmugam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@devcorpio devcorpio merged commit 9785834 into main Sep 19, 2023
18 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom serializer for Unhandled promise rejection: <object>
2 participants