search

mscdex / node-imap

An IMAP client module for node.js.
MIT License
2.17k stars 382 forks source link

Ending the connection results in random floating socket errors #779

Open iby opened 5 years ago

iby commented 5 years ago

I'm trying to close a no longer needed connection, and sometimes this leads to different socket errors.

const imap = new IMAP(config);

imap.once("close", (failed: boolean) => {
    console.log(`closed, failed: ${failed}`);
});

imap.once("error", (e: Error) => {
    console.log(e);
});

imap.once("ready", () => {
    imap.end();
});

imap.connect();

image

Stack ``` anonymous(), MailboxUtilityTest.ts:32 onceWrapper(), events.js:291 emit(), events.js:203 _onError(), Connection.js:151 emit(), events.js:203 TLSSocket._emitTLSError(), _tls_wrap.js:748 onerror(), _tls_wrap.js:330 Async call from TLSWRAP init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 TLSSocket._wrapHandle(), _tls_wrap.js:514 TLSSocket(), _tls_wrap.js:412 connect(), _tls_wrap.js:1405 anonymous(), Connection.js:1708 Object..Connection._resTagged(), Connection.js:1535 anonymous(), Connection.js:194 emit(), events.js:203 Object..Parser._resTagged(), Parser.js:175 Object..Parser._parse(), Parser.js:139 Object..Parser._tryread(), Parser.js:82 Parser._cbReadable(), Parser.js:53 emit(), events.js:203 emitReadable_(), _stream_readable.js:565 processTicksAndRejections(), task_queues.js:77 Async call from TickObject init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 emitInitScript(), async_hooks.js:341 TickObject(), task_queues.js:102 nextTick(), task_queues.js:130 emitReadable(), _stream_readable.js:557 addChunk(), _stream_readable.js:305 readableAddChunk(), _stream_readable.js:276 Readable.push(), _stream_readable.js:210 onStreamRead(), stream_base_commons.js:166 Async call from TCPWRAP init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 Socket.connect(), net.js:872 Object..Connection.connect(), Connection.js:286 anonymous(), MailboxUtilityTest.ts:39 anonymous(), MailboxUtilityTest.ts:23 asyncJestTest(), jasmineAsyncInstall.js:102 anonymous(), queueRunner.js:43 mapper(), queueRunner.js:26 anonymous(), queueRunner.js:73 Async call from Promise.then anonymous(), queueRunner.js:73 queueRunner(), queueRunner.js:72 queueRunnerFactory(), Env.js:277 execute(), Spec.js:169 fn(), treeProcessor.js:69 anonymous(), queueRunner.js:43 mapper(), queueRunner.js:26 anonymous(), queueRunner.js:73 Async call from Promise.then anonymous(), queueRunner.js:73 queueRunner(), queueRunner.js:72 queueRunnerFactory(), Env.js:277 anonymous(), treeProcessor.js:79 asyncGeneratorStep(), treeProcessor.js:13 _next(), treeProcessor.js:33 anonymous(), treeProcessor.js:38 anonymous(), treeProcessor.js:30 fn(), treeProcessor.js:89 anonymous(), queueRunner.js:43 mapper(), queueRunner.js:26 anonymous(), queueRunner.js:73 Async call from Promise.then anonymous(), queueRunner.js:73 queueRunner(), queueRunner.js:72 queueRunnerFactory(), Env.js:277 anonymous(), treeProcessor.js:79 asyncGeneratorStep(), treeProcessor.js:13 _next(), treeProcessor.js:33 anonymous(), treeProcessor.js:38 anonymous(), treeProcessor.js:30 fn(), treeProcessor.js:89 treeProcessor(), treeProcessor.js:122 anonymous(), Env.js:343 asyncGeneratorStep(), Env.js:36 _next(), Env.js:56 anonymous(), Env.js:61 anonymous(), Env.js:53 anonymous(), Env.js:388 anonymous(), index.js:203 asyncGeneratorStep(), index.js:27 _next(), index.js:47 anonymous(), index.js:52 anonymous(), index.js:44 _jasmine(), index.js:207 jasmine2(), index.js:60 anonymous(), runTest.js:385 asyncGeneratorStep(), runTest.js:161 _next(), runTest.js:181 Async call from Promise.then asyncGeneratorStep(), runTest.js:170 _next(), runTest.js:181 anonymous(), runTest.js:186 anonymous(), runTest.js:178 _runTestInternal(), runTest.js:445 runTestInternal(), runTest.js:225 anonymous(), runTest.js:460 asyncGeneratorStep(), runTest.js:161 _next(), runTest.js:181 anonymous(), runTest.js:186 anonymous(), runTest.js:178 _runTest(), runTest.js:481 runTest(), runTest.js:449 anonymous(), index.js:159 asyncGeneratorStep(), index.js:61 _next(), index.js:81 Async call from Promise.then asyncGeneratorStep(), index.js:70 _next(), index.js:81 anonymous(), index.js:86 anonymous(), index.js:78 Async call from Promise.then anonymous(), index.js:150 anonymous(), index.js:11 run(), index.js:10 anonymous(), index.js:62 anonymous(), index.js:148 anonymous(), index.js:146 asyncGeneratorStep(), index.js:61 _next(), index.js:81 anonymous(), index.js:86 anonymous(), index.js:78 _createInBandTestRun(), index.js:167 anonymous(), index.js:123 asyncGeneratorStep(), index.js:61 _next(), index.js:81 anonymous(), index.js:86 anonymous(), index.js:78 runTests(), index.js:137 anonymous(), TestScheduler.js:353 asyncGeneratorStep(), TestScheduler.js:131 _next(), TestScheduler.js:151 Async call from Promise.then asyncGeneratorStep(), TestScheduler.js:140 _next(), TestScheduler.js:151 anonymous(), TestScheduler.js:156 anonymous(), TestScheduler.js:148 scheduleTests(), TestScheduler.js:387 anonymous(), runJest.js:418 asyncGeneratorStep(), runJest.js:148 _next(), runJest.js:168 ```

image

Stack ``` anonymous(), MailboxUtilityTest.ts:32 onceWrapper(), events.js:291 emit(), events.js:203 _onError(), Connection.js:151 emit(), events.js:203 emitErrorNT(), destroy.js:91 emitErrorAndCloseNT(), destroy.js:59 processTicksAndRejections(), task_queues.js:77 Async call from TickObject init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 emitInitScript(), async_hooks.js:341 TickObject(), task_queues.js:102 nextTick(), task_queues.js:130 anonymous(), destroy.js:43 Socket._destroy(), net.js:595 destroy(), destroy.js:37 onStreamRead(), stream_base_commons.js:183 Async call from TLSWRAP init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 TLSSocket._wrapHandle(), _tls_wrap.js:514 TLSSocket(), _tls_wrap.js:412 connect(), _tls_wrap.js:1405 anonymous(), Connection.js:1708 Object..Connection._resTagged(), Connection.js:1535 anonymous(), Connection.js:194 emit(), events.js:203 Object..Parser._resTagged(), Parser.js:175 Object..Parser._parse(), Parser.js:139 Object..Parser._tryread(), Parser.js:82 Parser._cbReadable(), Parser.js:53 emit(), events.js:203 emitReadable_(), _stream_readable.js:565 processTicksAndRejections(), task_queues.js:77 Async call from TickObject init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 emitInitScript(), async_hooks.js:341 TickObject(), task_queues.js:102 nextTick(), task_queues.js:130 emitReadable(), _stream_readable.js:557 addChunk(), _stream_readable.js:305 readableAddChunk(), _stream_readable.js:276 Readable.push(), _stream_readable.js:210 onStreamRead(), stream_base_commons.js:166 Async call from TCPWRAP init(), inspector_async_hook.js:21 emitInitNative(), async_hooks.js:134 Socket.connect(), net.js:872 Object..Connection.connect(), Connection.js:286 anonymous(), MailboxUtilityTest.ts:39 anonymous(), MailboxUtilityTest.ts:23 asyncJestTest(), jasmineAsyncInstall.js:102 anonymous(), queueRunner.js:43 mapper(), queueRunner.js:26 anonymous(), queueRunner.js:73 Async call from Promise.then anonymous(), queueRunner.js:73 queueRunner(), queueRunner.js:72 queueRunnerFactory(), Env.js:277 execute(), Spec.js:169 fn(), treeProcessor.js:69 anonymous(), queueRunner.js:43 mapper(), queueRunner.js:26 anonymous(), queueRunner.js:73 Async call from Promise.then anonymous(), queueRunner.js:73 queueRunner(), queueRunner.js:72 queueRunnerFactory(), Env.js:277 anonymous(), treeProcessor.js:79 asyncGeneratorStep(), treeProcessor.js:13 _next(), treeProcessor.js:33 anonymous(), treeProcessor.js:38 anonymous(), treeProcessor.js:30 fn(), treeProcessor.js:89 anonymous(), queueRunner.js:43 mapper(), queueRunner.js:26 anonymous(), queueRunner.js:73 Async call from Promise.then anonymous(), queueRunner.js:73 queueRunner(), queueRunner.js:72 queueRunnerFactory(), Env.js:277 anonymous(), treeProcessor.js:79 asyncGeneratorStep(), treeProcessor.js:13 _next(), treeProcessor.js:33 anonymous(), treeProcessor.js:38 anonymous(), treeProcessor.js:30 fn(), treeProcessor.js:89 treeProcessor(), treeProcessor.js:122 anonymous(), Env.js:343 asyncGeneratorStep(), Env.js:36 _next(), Env.js:56 anonymous(), Env.js:61 anonymous(), Env.js:53 anonymous(), Env.js:388 anonymous(), index.js:203 asyncGeneratorStep(), index.js:27 _next(), index.js:47 anonymous(), index.js:52 anonymous(), index.js:44 _jasmine(), index.js:207 jasmine2(), index.js:60 anonymous(), runTest.js:385 asyncGeneratorStep(), runTest.js:161 _next(), runTest.js:181 Async call from Promise.then asyncGeneratorStep(), runTest.js:170 _next(), runTest.js:181 anonymous(), runTest.js:186 anonymous(), runTest.js:178 _runTestInternal(), runTest.js:445 runTestInternal(), runTest.js:225 anonymous(), runTest.js:460 asyncGeneratorStep(), runTest.js:161 _next(), runTest.js:181 anonymous(), runTest.js:186 anonymous(), runTest.js:178 _runTest(), runTest.js:481 runTest(), runTest.js:449 anonymous(), index.js:159 asyncGeneratorStep(), index.js:61 _next(), index.js:81 Async call from Promise.then asyncGeneratorStep(), index.js:70 _next(), index.js:81 anonymous(), index.js:86 anonymous(), index.js:78 Async call from Promise.then anonymous(), index.js:150 anonymous(), index.js:11 run(), index.js:10 anonymous(), index.js:62 anonymous(), index.js:148 anonymous(), index.js:146 asyncGeneratorStep(), index.js:61 _next(), index.js:81 anonymous(), index.js:86 anonymous(), index.js:78 _createInBandTestRun(), index.js:167 anonymous(), index.js:123 asyncGeneratorStep(), index.js:61 _next(), index.js:81 anonymous(), index.js:86 anonymous(), index.js:78 runTests(), index.js:137 anonymous(), TestScheduler.js:353 asyncGeneratorStep(), TestScheduler.js:131 _next(), TestScheduler.js:151 ```

It's a pain to manage those locally, since the failures are inconsistent and close handler comes through with failed: false as if there's no error.

One way to manage these is to use a complex once-like pattern, which would install and remove listeners. Otherwise, by simply removing all error listeners prior calling end, which doesn't feel like the right thing to do.

mscdex commented 5 years ago

The argument passed to 'close' event handlers is merely what's passed from node core and is more of a historical thing.

I'm not sure I understand the problem though, you should always handle unexpected errors. Why are you concerned about having to removing the event handlers?

iby commented 5 years ago

I'm not very familiar with the underlying api and what supposed to be happening, but pretty sure it shouldn't end with an error in this case. At the same time I do want to know of any errors, but you are right, this is more of an ideological case than practical.

I'm using this in a cli app and wrapping the module calls into promises to make use of the async and await. For some reason the code fails when the error callback fires and rejects the already resolved promise. From what I understand, failing the already resolved promise shouldn't affect it, but it somehow causes the cli app to fail. And without closing the stream the app never finishes.

I ended up handling those cases on per-promise basis, which seems to work smoothly so far, but it's a lot of boilerplate for what should work out of the box. From what you're saying I understand it's not the project related issue? Should it be reported else where?

mscdex commented 5 years ago

If the issue is only present with Promises, then that's outside the scope of this project.

iby commented 5 years ago

The messed up promise is the consequence, not the cause. The problem is with the imap package (or a dependency) randomly sending an error event after the close event when end()ing it.

The isolated code above illustrates this. Like said, this doesn't happen all the time, but often and consistently with different services.

PRR24 commented 3 years ago

Not sure if it is connected, but I have a software running against two email service providers. Calling imap.end() triggers "end" and "close" events if connected to server #1, but only "close" event in case of server #2. Only difference is username, password and email server name.