strongloop / loopback-component-storage

Storage component for LoopBack.
Other
130 stars 155 forks source link

Infinite loop when generating metadata for files in Container #289

Closed juanvillegas closed 4 years ago

juanvillegas commented 4 years ago

Steps to reproduce

We are using Loopback 3 with Node 12.14.1. Hard to say, it probably doesn't affect everyone as I haven't seen any other issues reporting the same thing here. We noticed a Call stack exceeded error happening from time to time in our server, and after debugging we discovered the response of the following api request returns an enormous, recursive structure that eventually blocks the server causing a gateway error. The api request is: https://www.smylen.com/api/containers/DoctorLocations_294/files and the response is:

image

Where _DoctorLocations295 is the name of the container, and /files contains only a couple of images.

Current Behavior

We get a massive, apparently recursive, structure as displayed in the next screen (it's displayed only partially, but note the response is 3mb of text and the json structure is recursively containing the same information over and over, until eventually it breaks the server) image

Note: rebooting the server clears the "garbage" and for some time it works fine, but after a few minutes it starts again.

We were able to "patch" it by changing lines 46 and 47 in the node_modules/loopback-component-storage/lib/factory.js file (see that we "disconnected" _rawMetadata and manually copied the receiver object into _metadata):

image

Expected Behavior

The normal structure with metadata for each of the files.

Link to reproduction sandbox

Unfortunately, we cannot provide a way to reproduce it at the moment as our production server was "patched" with the previously mentioned code. We could create a replica of our server somewhere if you really wanted to see it, but maybe someone has stumbled upon this before and we can save that step? We use Amazon so we can easily boot a new server, but still it will take some time to secure it and bla bla.

Additional information

node -e 'console.log(process.platform, process.arch, process.versions.node)'

Output is: linux x64 12.14.1

npm ls --prod --depth 0 | grep loopback

Output is: +-- loopback@3.26.0 +-- loopback-boot@2.28.0 +-- loopback-component-explorer@4.3.1 +-- loopback-component-storage@3.6.3 +-- loopback-connector-mysql@4.2.0 +-- loopback-connector-rest@2.1.0

It is running on the the node:12.14.1 docker base image.

Related Issues

None

Any help/pointers would be appreciated. Juan

emonddr commented 4 years ago

@juanvillegas , thank you for letting us know.

1) If you keep the lb3 app outside of a Docker container (running on your laptop), does the same error occur?

2) Please provide us with a small app that replicates the error (no production data or credentials) thx.

juanvillegas commented 4 years ago

1) Yes, this was also happening a few months ago when our app was in a regular Ec2 server. 2) I'll talk with Operations team to see what can they do.

hacksparrow commented 4 years ago

@juanvillegas since we don't know what other components are involved in requesting and parsing the request, it would difficult to say whether the problem lies.

If this issue can be reproduced in a standalone LB3 app, we can definitely fix the bug.

emonddr commented 4 years ago

@juanvillegas, please provide a standalone app in LB3 that displays this problem. thanks.

emonddr commented 4 years ago

Closing due to inactivity. If you are still running into problems, feel free to leave a comment and I will reopen the issue.

juanvillegas commented 4 years ago

@emonddr Sorry for the no answer below. I'm unfortunately unable to provide a standalone application as that would imply exposing the whole project. However, I was finally able to reproduce the issue in my local environment this weekend. My local environment is a standard OSx, different from our Production environment, which runs Ubuntu. For this reason, I'm surprised no one has experienced this before, so I tend to believe this has to be rooted at our application code, although it is hard to say where as it is a pretty standard usage of the component. Anyway, I'm happy to schedule a short video session to quickly demo the issue if you are interested in checking this out. Maybe some ideas pop up after seeing what's going on. I will continue to debug on my end.

Thanks! Juan Villegas

juanvillegas commented 4 years ago

Some more details after spending a couple hours researching this. We managed to track it down to this function call: https://github.com/strongloop/loopback-component-storage/blob/master/lib/factory.js#L40 It's hard to tell exactly what's going in the class as this is library code and thus would take some time for me to analyze. But one thing is certain: m1.call enters into a deep loop, where details are passed over and over in the call chain. The this reference keeps changing to receiver, which keeps copying details until for some reason it stops. When it starts returning results, this.metadata will point to the returned object, thus creating a huge recursive structure. The recursive structure eventually leads to huge responses in the order of MBs that the browser can't parse. Our previous solution consisted of manually altering lines 46 and 47 (https://github.com/strongloop/loopback-component-storage/blob/master/lib/factory.js#L47) to do an explicit copy rather than a reference. This prevents the browser from getting a non parseable response. However, the recursive loop eventually absorbs all the memory and throws an exception that forces the web server to restart.

emonddr commented 4 years ago

@deepakrkris or @hacksparrow ^. Thx.

hacksparrow commented 4 years ago

@juanvillegas can you open a PR with the fix? We can continue the discussion and progress there.

juanvillegas commented 4 years ago

@hacksparrow Thanks for your response, I've just created a PR with the line we commented out for you to have a look here: https://github.com/strongloop/loopback-component-storage/pull/291.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] commented 4 years ago

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.