chadxz / imap-simple

Wrapper over node-imap, providing a simpler api for common use cases
MIT License
243 stars 80 forks source link

ECONNRESET issue. #66

Closed shashankklenty closed 4 years ago

shashankklenty commented 5 years ago

Library Version: 4.3.0

Error:

events.js:174
throw er; // Unhandled 'error' event
^

Error: read ECONNRESET
at TCP.onStreamRead (internal/stream_base_commons.js:111:27)
Emitted 'error' event at:
at Connection.<anonymous> (/app/node_modules/imap-simple/lib/imapSimple.js:39:14)
at Connection.emit (events.js:189:13)
at TLSSocket._onError (/app/node_modules/imap/lib/Connection.js:151:10)
at TLSSocket.emit (events.js:189:13)
at TLSSocket._emitTLSError (_tls_wrap.js:608:10)
at JSStreamWrap.wrap.on (_tls_wrap.js:551:36)
at JSStreamWrap.emit (events.js:189:13)
at Socket.JSStreamWrap.stream.on (internal/wrap_js_stream.js:50:38)
at Socket.emit (events.js:189:13)
at emitErrorNT (internal/streams/destroy.js:82:8)
at emitErrorAndCloseNT (internal/streams/destroy.js:50:3)
at process._tickCallback (internal/process/next_tick.js:63:19)

Issue Description I am currently using this library to track replies from various different imap servers in production and it has been working perfectly for me. I have only very recently started facing this problem where the ECONNRESET error will suddenly get thrown while the application is idle which causes a crash. I have tried establishing the imap connection in production while debugging is enabled (debug: console.log) however all the connections made are successfully ended without any problems and no other errors occur in my code as well.

My ImapSimpleOptions object is as follows:

const config: imaps.ImapSimpleOptions = {
            imap: {
                user: ...,
                password: ...,
                host: ...,
                port: ...,
                tls: (port !== 143),
                authTimeout: 25000,
                tlsOptions: { rejectUnauthorized: false },
                keepalive: false,
                debug: console.log
            }
        };

My Workflow:

  1. Validate if credentials provided by my user is correct by attempting to .connect() and then .openBox("INBOX") using the same config object mentioned above. After which, I .end() the connection.
  2. If the previous step succeeded without any errors, I carry out the same flow again and after opening the inbox I use .search() API to query the data I need. Once received, again, I .end() the connection. For safety, in all my catch blocks within the scope of .connect(), I have also added .end() calls.

I am not effectively able to reproduce this in a development environment but after giving your code a quick read, I can see that here you have created an 'error' event listener that might not be returning from there as self.ending would not be true. Could you please check if removing that flag check could solve this error that is being thrown?

Another suggestion: Can you please allow us to define an 'error' listener of our own if we know what we are doing?

shashankklenty commented 5 years ago

Ignore the suggestion, I was able to create an "error" event listener by doing something like:

imaps.connect(options)
    .then(connection => {
        connection.on("error", function() {
            console.log("An error occured. This should handle it?");
        });
        return connection.openBox(folderName).then(() => { an so on...

This has helped me prevent my application from crashing in production however it would be nice if we could investigate a solution for this.

OlivierCuyp commented 4 years ago

@shashankklenty I had the same issue with append command. The issue is that imap-simple is not handling the end event when performing actions. In case the server (in my case Office 365) close the connection in the middle of an action node-imap doesn't generate an error event but just the end event. So the append callback is never called and the Promise never finish.

You can see that there is no event handled in the append:

https://github.com/chadxz/imap-simple/blob/master/lib/imapSimple.js#L435

It should have the same events handling like in the connect method:

https://github.com/chadxz/imap-simple/blob/master/lib/imapSimple.js#L548

This should be done in all actions.

shashankklenty commented 4 years ago

@OlivierCuyp Thanks for the tip, I'll be forking this library soon to implement all the event listeners, will inform you when its ready in case you're interested :)

OlivierCuyp commented 4 years ago

@shashankklenty Why don't you do a PR instead of a fork ? Either way, let me know ;)

shashankklenty commented 4 years ago

@OlivierCuyp I wasnt sure if the PR would get accepted anytime soon. I've added the 'close' event handler in the search and fetch method. I'll be adding it to the others when i have the time, unless you'd like to raise a PR. Here ya go. https://github.com/klenty/imap

Its on npm as well, @klenty/imap

OlivierCuyp commented 4 years ago

@shashankklenty Thanks for the link. But I still think the best option would be to make a PR on this repo to help all other users of this package.

shashankklenty commented 4 years ago

Alright will raise a PR asap. I still plan to stick to my fork though.

OlivierCuyp commented 4 years ago

Since @chadxz doesn't seems to really care about this package (see this) maybe you could ask him to become maintainer of this repo or take it over.

shashankklenty commented 4 years ago

That comment was the reason why I forked. Developers at my organization would be actively maintaining the fork to the best of their abilities. I agree, the right option was to directly PR since it would help everyone else but since this was an urgent issue in production for us, I had to fork it.

chadxz commented 4 years ago

I'm happy to give maintainership. I've just been waiting on someone to show interest that wasn't going to be malicious.

OlivierCuyp commented 4 years ago

@chadxz I think your package has some good traction and it would be sad to let it die ... So building a nice group of maintainers would be nice.

DTX-92 commented 4 years ago

@shashankklenty how did you get that error stack trace? In mine I don't see anything about imap-simle.. And it was hard to realise, that error was related to imap =\ image