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

change formatError to not set ecsFields.err if the given err isn't an Error #66

Open
trentm opened this issue Mar 18, 2021 · 1 comment
Labels
agent-nodejs Make available for APM Agents project planning.

Comments

@trentm
Copy link
Member

trentm commented Mar 18, 2021

Currently formatError in the helpers package does this:

function formatError (ecsFields, err) {
  if (!(err instanceof Error)) {
    ecsFields.err = err  // <--- this line
    return
  }

I.e. if err is an Error, then it will fill in ecsFields.error. Otherwise it will set ecsFields.err. But "err" isn't a spec'd ECS field, so it shouldn't do that. It should be up to the callers to handle this. This would also better match formatHttpRequest and formatHttpResponse which do not set ecsFields.req or .res if they cannot process the given value.

This will be a breaking change.

@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Mar 18, 2021
@trentm
Copy link
Member Author

trentm commented Oct 25, 2023

Note that as of recently #158, the winston formatter is no longer using helpers.formatError() partly for this reason and partly to add other fields: .cause and any enumerable properties on the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

No branches or pull requests

1 participant