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

Memory Leak when Handling 502 Errors #3179

Closed
astormnewrelic opened this issue Aug 7, 2020 · 16 comments · Fixed by #3771
Closed

Memory Leak when Handling 502 Errors #3179

astormnewrelic opened this issue Aug 7, 2020 · 16 comments · Fixed by #3771
Projects
Milestone

Comments

@astormnewrelic
Copy link

astormnewrelic commented Aug 7, 2020

Describe the bug

We've discovered what appears to be a memory leak in axios when handling HTTP requests that result in a 502 error.

To Reproduce

  1. Run this small program in a terminal window (sets up an endpoint to call)

  2. Run the program whose source is provided below

    $ node main.js > /dev/null
    
  3. Monitor memory usage of the node process from program Support file uploads #2

Expected Behavior: Memory usage stays constant

Actual Behavior: Memory use slowly grows over time

//File: main.js
const axios = require('axios')

// localhost:3001/bad
const client = axios.create({
  baseURL: 'http://localhost:3001'
})

const work = async () => {
  try {
    await client.get('/bad')
    console.log('did it')
  } catch (error) {
    console.log('an error')
  }
}

const main = () => {
  setInterval(work, 0)
}
main()

Expected behavior

We expect the memory use of the node process should stay consistent and not grow

Environment

  • Axios Version 0.19.2
  • Adapter: HTTP
  • Node.js Version: v12.18.2
  • OS: 10.14.6

Additional context/Screenshots

We maintain New Relic's NodeJS agent. We had a custom come to us reporting a memory leak in the Node Agent, with a reproduction here. Like a lot of the leaks reported to us, this appears to be a case of a small leak in an upstream package (axois) being made much worse by the steps the New Relic's Node Agent needs to take in order to instrument it's thing.

@chinesedfan
Copy link
Collaborator

Can you give full codes of main.js? Looks like it includes an express server. Then the memory leaking is hard to be determined by axios or express.

@astormnewrelic
Copy link
Author

astormnewrelic commented Aug 15, 2020

@chinesedfan Thank you for taking a look. Also -- d'oh -- my apologies. I appear to have copy and pasted the wrong program into my ticket description. I have pasted the actual reproduction into the description, and I will include it below as well.

const axios = require('axios')

// localhost:3001/bad -- 
const client = axios.create({
  baseURL: 'http://localhost:3001'
})

const work = async () => {
  try {
    await client.get('/bad')
    console.log('did it')
  } catch (error) {
    console.log('an error')
  }
}

const main = () => {
  setInterval(work, 0)
}
main()

This program isolates the problem so that axios is the only third party library in use.

@astormnewrelic
Copy link
Author

@chinesedfan were you able to reproduce with the above program, or are you not seeing the problem we're seeing? Is there anything we can do to help move this along?

@chinesedfan
Copy link
Collaborator

I have a question about the example first, why you need setInterval with timeout=0. If you commented the line with axios, will its memory still keeps growing?

@astormnewrelic
Copy link
Author

astormnewrelic commented Aug 29, 2020

@chinesedfan If we comment out the axios line the memory will not keep growing, it stabilizes.

We're using setInterval in the above example to call the work function over and over again in order to show that the call to axios appears to be leaking memory in the thrown exception case. i.e. The best way to show it wasn't express was to remove express :)

Is there's a better way for us to demonstrate the memory leak for you? Please let us know and we'll be happy to assist.

@sammccord
Copy link

Seeing this as well, would love to see an update :)

@astorm
Copy link

astorm commented Sep 16, 2020

@chinesedfan any updates on this or anything @sammccord and I could do to help move this along?

@chinesedfan
Copy link
Collaborator

Sorry, I don't have much experiences about memory leaking. There are some similar issues in our issue list. Hope the community can find out the reason with an easy understanding report.

@astorm
Copy link

astorm commented Sep 17, 2020

Thank you @chinesedfan -- so if I understand you correctly in addition to seeing the behavior reproduced, you'll need someone that can actually get into the axios internals and describe what's going on that leads to the leak. @sammccord -- is that the sort of thing you might be up for?

@github-actions
Copy link
Contributor

Hello! 👋

This issue is being automatically marked as stale because it has not been updated in a while. Please confirm that the issue is still present and reproducible. If no updates or new comments are received the issue will be closed in a few days.

Thanks.

@astorm
Copy link

astorm commented Oct 19, 2020

Still present, still reproducible with the steps listed above.

@pgcalixto
Copy link

Hey, guys, I saw there has been recent updates on this issue (@astorm said it is still reproducible), I'd like to work on this. Is anyone else already tackling this problem?

@chinesedfan
Copy link
Collaborator

@pgcalixto Sure. Feel free to take it, as well as any other issues of axios. This repository requires help from the community urgently.

@jasonsaayman
Copy link
Member

jasonsaayman commented Mar 23, 2021

Please check if this has been fixed with #3694

@astormnewrelic
Copy link
Author

Happy to check @jasonsaayman -- is that fix in a release yet? If not is there a canonical way to run axios from the master branch (is it enough to point NPM at the master branch of this repo, or is a build step required, etc?)

@jasonsaayman
Copy link
Member

@astormnewrelic it seems that the error actually has to do with the follow-redirects package so the issue persists. To answer your question you can do the following npm install --save ../path/to/mymodule.

@jasonsaayman jasonsaayman linked a pull request Apr 29, 2021 that will close this issue
@jasonsaayman jasonsaayman added this to the v0.22.0 milestone Apr 29, 2021
@jasonsaayman jasonsaayman added this to To do in v0.22.0 via automation Apr 29, 2021
v0.22.0 automation moved this from To do to Done Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v0.22.0
  
Done
Development

Successfully merging a pull request may close this issue.

6 participants