Skip to content

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.

@devcorpio devcorpio linked an issue Sep 18, 2023 that may be closed by this pull request
}

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants