Skip to content

Test if connect() throw an unhandled exception#13

Open
peernohell wants to merge 2 commits intopipedrive:masterfrom
peernohell:patch-1
Open

Test if connect() throw an unhandled exception#13
peernohell wants to merge 2 commits intopipedrive:masterfrom
peernohell:patch-1

Conversation

@peernohell
Copy link
Copy Markdown

I just find an issue with connect. When called on certain server the connection failed.
It throw correctly the error but an unhandle promise is also rejected. it should not be cough by the system and should be normally trapped.
I didn't investigate much more at the moment as I spend more time to reproduce the issue and create a test case.
I will come back with more detail to understand why connecting on that server trigger the unhandledRejection event.

If you have any suggestion on modification or code to adjust please don't hesitate 🙏

I just find an issue with connect. When called on certain server the connection failed.
It throw correctly the error but an unhandle promise is also rejected. it should not be cough by the system and should be normally trapped.
I didn't investigate much more at the moment as I spend more time to reproduce the issue and create a test case.
I will come back with more detail to understand why connecting on that server trigger the `unhandledRejection` event.
@peernohell
Copy link
Copy Markdown
Author

If found what's happened.
With that server connection seems to be establish but as it's not an IMAP server, the capability command failed.
The issue is in the function openConnection when updateCapability capability there's no catch and if the function failed then an unhandled exception is throw.

the fix is pretty easy. We just need to replace the line

 this.updateCapability()
            .then(() => resolve(this._capability))

by

 this.updateCapability()
            .then(() => resolve(this._capability), reject)

Then the issue stop to produce.

When `updateCapability` failed, the reject is not call and an unhandled exception is throw
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.

1 participant