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

catch harvest callback error #275

Closed
wants to merge 2 commits into from
Closed

Conversation

joway
Copy link

@joway joway commented Apr 1, 2019

CHANGE LOG

fix crush when Error: Callback was already called.

INTERNAL LINKS

NOTES

@joway
Copy link
Author

joway commented Apr 1, 2019

image

When we enable newrelic proxy option, it seems sometimes will have ECONNRESET error, and the nodejs process will crush.

So we need to add try catch on callback function to prevent it.

Copy link
Member

@michaelgoin michaelgoin left a comment

Choose a reason for hiding this comment

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

@joway thank you for the contribution! A try/catch may not be how we want to solve this long-term, as we need to track down why this is happening and prevent it, but this is a good starting point to try to narrow-in on the issue.

Would it be possible to create a test to reproduce the issue as you understand it? A module such as nock can be helpful to trigger such issues. We use it in a few places in our tests, which can be used for examples such as test/unit/agent/harvest.test.js. This would help determine if the solution is ideal and also prevent future regressions.

Thank you!

@michaelgoin
Copy link
Member

Hi!

We pushed a patch in version 5.6.4 that prevents this issue from throwing errors and also attempts to capture where the multiple calls were originating from. Using a version of the agent ^5.6.4, if you see the new log message indicating the attempt at multiple callback invocations, please let us know as it will help us further track down the underlying issue.

I'll be closing this PR, since we won't be accepting the immediate changes, but we appreciate the help.

Thank you,

Michael

@joway joway deleted the try_catch_callback branch May 27, 2019 02:39
@joway
Copy link
Author

joway commented May 27, 2019

Hi, @michaelgoin ! Thanks for your great work!

bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this pull request Apr 19, 2024
bizob2828 pushed a commit to bizob2828/node-newrelic that referenced this pull request Apr 23, 2024
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.

None yet

2 participants