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

Node exits with code 0 when uploading a zero byte file to AWS S3 #46182

Closed
bes opened this issue Jan 12, 2023 · 15 comments
Closed

Node exits with code 0 when uploading a zero byte file to AWS S3 #46182

bes opened this issue Jan 12, 2023 · 15 comments

Comments

@bes
Copy link

bes commented Jan 12, 2023

Version

v19.4.0

Platform

Darwin hasu.local 22.1.0 Darwin Kernel Version 22.1.0: Sun Oct 9 20:14:54 PDT 2022; root:xnu-8792.41.9~2/RELEASE_X86_64 x86_64

Subsystem

http

What steps will reproduce the bug?

POC with instructions https://github.com/bes/aws-sdk-js-v3-s3-exit-0
Previous discussion aws/aws-sdk-js-v3#4332

How often does it reproduce? Is there a required condition?

On my 16" 2019 MacBook Pro with 2,6 GHz 6-Core Intel Core i7 I am able to reproduce this issue with 100% reproducibility within a few seconds of starting the script.

What is the expected behavior?

The script should run forever, or until Ctrl+C is issued.
As long as the script is running, it prints START before a call to the AWS S3 SDK's s3.send method, and END immediately after.

What do you see instead?

After a few seconds, meaning 1 to 30 http calls, node exits with exit code 0

yarn run poc
yarn run v1.22.19
$ TS_NODE_PROJECT="tsconfig.json" ts-node build.ts
START
DONE
START
DONE
START
DONE
START
DONE
START
DONE
START
DONE
START
DONE
START
DONE
START
✨  Done in 1.91s.

As you can see, it prints START and then exits without printing DONE.

Additional information

No response

@bnoordhuis
Copy link
Member

Do you have a test case that doesn't depend on third-party dependencies? I read through your PoC and the linked issue but I'm not convinced yet it's a node issue (and I don't plan on going over AWS's SDK unless they pay me to.)

@bes
Copy link
Author

bes commented Jan 12, 2023

@bnoordhuis Thanks, I understand and sympathize.
@RanVaknin Do you know if there are nodejs developers on AWS' payroll that can look at this?

In the meanwhile I will try to see if I can create a POC without AWS SDK.

@marco-ippolito
Copy link
Member

marco-ippolito commented Jan 12, 2023

I can have a look, in the meantime if you could add a repro without aws-sdk and in js would be great 😁

@marco-ippolito
Copy link
Member

marco-ippolito commented Jan 12, 2023

In my first attempt debugging this, I found out:

break in build.js:124
 122 process.on("exit", function () {
>124     debugger;
 125 });
debug> bt
#0 (anonymous) build.js:124:4
#1 emit node:events:519:27
debug> 

node/lib/events.js

Lines 516 to 524 in 91ca2d4

if (typeof handler === 'function') {
const result = handler.apply(this, args);
// We check if result is undefined first because that
// is the most common case so we do not pay any perf
// penalty
if (result !== undefined && result !== null) {
addCatch(this, result, type, args);

might be a handler from the aws-sdk library

@RanVaknin
Copy link

Hi,

I think it's worth mentioning that #22088 might be related, and has examples of reproducing without the aws dependency.

Thanks,
Ran

@bnoordhuis
Copy link
Member

If your issue is #22088 then that's something you need to fix in your SDK. It's not a node bug.

@marco-ippolito
Copy link
Member

marco-ippolito commented Jan 13, 2023

break in build.js:135
 133 });
 134 process.on("beforeExit", function () {
>135     debugger;
 136 });
 137 
debug> bt
#0 (anonymous) build.js:135:4
#1 emit node:events:519:27

I think this issue is related to #22088, the execution stops with exit code 0 and beforeExit is invoked.
As documented:
The 'beforeExit' event is emitted when Node.js empties its event loop and has no additional work to schedule.

The 'beforeExit' event is not emitted for conditions causing explicit termination, such as calling process.exit() or uncaught exceptions.

even if it's weird, as stated by @bnoordhuis, it might not be a bug, I think the issue has to be searched inside aws-sdk.

@bnoordhuis
Copy link
Member

As there is no reason to believe this is a bug inside node itself, I'm going to go ahead and close out the issue.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Jan 18, 2023
@marco-ippolito
Copy link
Member

marco-ippolito commented Jan 18, 2023

As there is no reason to believe this is a bug inside node itself, I'm going to go ahead and close out the issue.

I'm still investigating on this with @ShogunPanda.
We are making sure it's not a bug on our side since there was a similar bug in the past #39671 (comment).

@trivikr trivikr reopened this Jan 18, 2023
@mohghaderi
Copy link

Could anyone please suggest a workaround?

It's been more than a year and I see no fix. It causes Electron to crash and we can't ship a product like that. aws-sdk v3 is useless for us due to this error, and v2 uses old version of libs that we don't want to risk security. It is nice that at least we have a workaround.

@marco-ippolito
Copy link
Member

marco-ippolito commented Jan 25, 2023

Could anyone please suggest a workaround?

It's been more than a year and I see no fix. It causes Electron to crash and we can't ship a product like that. aws-sdk v3 is useless for us due to this error, and v2 uses old version of libs that we don't want to risk security. It is nice that at least we have a workaround.

PR are welcome if you want to look into this.
It would be very useful if you could provide an example to reproduce without use of 3rd party libraries.
There is a discussion going on on aws repo: aws/aws-sdk-js-v3#4332
And I suspect it's not a bug of Node but wrong use of promises in the third party library.

@marco-ippolito
Copy link
Member

marco-ippolito commented Feb 16, 2023

As suspected this is a problem related to aws-sdk-v3 aws/aws-sdk-js-v3#4029 aws/aws-sdk-js-v3#4332 (comment) @bnoordhuis, not a node bug

@bnoordhuis
Copy link
Member

Thanks for the update, @marco-ippolito. Closing again then.

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale Feb 16, 2023
@mohghaderi
Copy link

Sorry couldn't reproduce outside of AWS SDK v3. It is fine on v2.

I still think that Node.js should have a way to handle such issues and don't let 3rd party libs to cause a crash. I understand that it may not be trivial to do it.

@marco-ippolito
Copy link
Member

marco-ippolito commented Feb 16, 2023

it is not a crash, it exits with code 0 as I stated here #46182 (comment), this is related to incorrect use of promises

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

No branches or pull requests

6 participants