-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
S3.putObject RangeError: Maximum call stack size exceeded node v0.10.15 #158
Comments
This looks like a duplicate of #150, which is not an issue in the SDK. Given that I was not able to reproduce this with v0.10.16 I would recommend upgrading Node.js, since it looks like a regression. I'm going to close this as a third-party issue. |
Noting here that I also saw this with node v0.10.26 on windows. |
@springmeyer can you provide a test case that reproduces the error for you? |
hi @lsegal - trying to create a reproducible testcase for the process.nextTick error, but having trouble. I don't have access to the machine where the warnings happened right now (since they were on a windows appveyor box - logs at https://ci.appveyor.com/project/BergWerkGIS/node-mapnik/build/1.0.17#L939). So I'm trying to replicate locally on OS X. In the process hitting a different issue, which perhaps is related? I'm seeing that with |
Added a readme and second test to https://gist.github.com/springmeyer/0a4e06bdec994db751b2. Neither replicate the process.nextTick issue. But I did see it once while working to create these test cases. |
@springmeyer I fixed the zero-buffer issue, investigating the second issue now. |
@springmeyer I've just pushed a fix for #248 which should fix both issues in your gist. The process.nextTick issue probably will not be resolved by this, though. |
@lsegal - excellent, thank you. I can confirm that both issues are also fixed in my local testing with latest master. As far as the process.nextTick mystery I'm planning on moving to multipart uploads (mapbox/node-pre-gyp#51) so I'm unlikely to hit this again. But I'll certainly post back if I do. Btw, are there any code samples for how to best implement multipart uploads beyond the docs you mention at #173 (comment)? |
In a last try to uncover the process.nextTick issue I noticed something else slightly odd (but maybe intended): If you pass an empty string for the var s3 = new AWS.S3();
var s3_obj_opts = { Body: fs.readFileSync('./e'),
Bucket: 'node-pre-gyp-tests',
Key: '',
};
s3.putObject(s3_obj_opts, function(err, resp){
if (err) throw err;
}); |
Thanks for finding all of these issues, @springmeyer. I've just resolved the last one in the above commit. |
Cheers. I'll create new issues for anything else I run into. Thanks for the fast fixes. |
I came across this issue when providing a large buffer as the Body to an This is definitely underlain by an upstream node.js issue, and it may be fixed in v0.12 nodejs/node-v0.x-archive#6065 (comment), but I think you may still fall into another trap like nodejs/node-v0.x-archive#7401 and nodejs/node-v0.x-archive#8291. I was able to work around the issue by wrapping my buffer in a readable stream, almost identically to aws-sdk's Here's a gist showing pass/fail cases of the |
Setting a highWaterMark seems like a reasonable and fairly simple thing to do on our part. Are there any downsides that I might be missing? |
Yeah, maybe, but I'm honestly not sure. The number that you choose does feel somewhat arbitrary. At first, I tried setting it to zero, and that seemed to effectively stop processing. I ended up setting it to 5MB, since I was confident that buffers of that size did not trigger the recursion errors. |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread. |
This bug does not exist with node v0.10.13, but does on v0.10.15, I have not tested v0.10.18
When doing putObject with a large file (100MB+), node issues this warning many time:
(node) warning: Recursive process.nextTick detected. This will break in the next version of node. Please use setImmediate for recursive deferral.
and then crashes:
RangeError: Maximum call stack size exceeded
Best quick-fix solution is to use a stream instead (probably what you wanted anyway):
The text was updated successfully, but these errors were encountered: