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

Fix a bug that cause UnhandledPromiseRejectionWarning on error #195

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SunriseFox
Copy link

@SunriseFox SunriseFox commented Sep 26, 2018

#194

This PR tries to fix a bug that cause stray unhandled promises when runs on NodeJS app even if client.onerror is specified.

The cause is when IMAP close with an error, for example, Socket closed unexpectedly!, it will try clean _clientQueue and _currentCommand with the same error. However, _currentCommand's callback simply rejects the promise, but nowhere to catch the promise rejection or even find it without reading hundreds lines of the code. Problem code is here: client.js#L718

Simply swallow the rejection is safe, as the original code simply throws it without giving the user any chance to catch it.

This PR breaks the test as the same error will not be rejected twice. I think this behavior should be ok.

@SunriseFox
Copy link
Author

Expected: all exceptions go through the onerror listener, if defined, no stray exceptions and no exception will be rejected twice. It may not be possible without rewriting a lot of parts of the code.

@jhoermann
Copy link

Any ideas why the test failed? Any chance that this will be pulled / pushed into master?

@SunriseFox
Copy link
Author

Any ideas why the test failed? Any chance that this will be pulled / pushed into master?

Hey, if you'd like to use this workaround, you could try something like npm install SunriseFox/emailjs-imap-client#master (but I suggest that you fork it yourself in case that I may change this in the future.

@cormip
Copy link

cormip commented Nov 21, 2018

Any progress on this fix?

2018-11-17 12:45:38-05:00(node:7) UnhandledPromiseRejectionWarning: Error: write after end
2018-11-17 12:45:38-05:00 at writeAfterEnd (_stream_writable.js:236:12)
2018-11-17 12:45:38-05:00 at TLSSocket.Writable.write (_stream_writable.js:287:5)
2018-11-17 12:45:38-05:00 at TLSSocket.Socket.write (net.js:707:40)
2018-11-17 12:45:38-05:00 at TCPSocket.send (/app/bundle/programs/server/npm/node_modules/emailjs-tcp-socket/dist/node-socket.js:129:20)
2018-11-17 12:45:38-05:00 at Imap.send (/app/bundle/programs/server/npm/node_modules/emailjs-imap-client/dist/imap.js:370:19)
2018-11-17 12:45:38-05:00 at Timeout._idleTimeout.setTimeout (/app/bundle/programs/server/npm/node_modules/emailjs-imap-client/dist/client.js:780:21)
2018-11-17 12:45:38-05:00 at ontimeout (timers.js:498:11)
2018-11-17 12:45:38-05:00 at tryOnTimeout (timers.js:323:5)
2018-11-17 12:45:38-05:00 at Timer.listOnTimeout (timers.js:290:5)
2018-11-17 12:45:38-05:00(node:7) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 2261)

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.

3 participants