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

Fixed time out issue #3436

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Fixed time out issue #3436

wants to merge 6 commits into from

Conversation

lnedry
Copy link
Contributor

@lnedry lnedry commented Jan 24, 2025

Fixes #

Changes proposed in this pull request:

  • removed redundant time out
  • fixed issue with promises
  • added several new tests

Checklist:

  • docs updated
  • tests updated
  • Changes updated

- Resolved issue with redundant timeout.
- Updated to use promises.
- added many new tests
- added many new tests
Added sinon to devDependencies. It is used in mail_from.is_resolvable tests.
Removed redundant time out
@msimerson
Copy link
Member

After this change, what happens when a DNS query takes 31+ seconds to resolve?

My memory thinks that it used to be something unpleasant, which was why that 29s timeout was added.

@lnedry
Copy link
Contributor Author

lnedry commented Jan 25, 2025

I'll work on it over the next few days.

@lnedry
Copy link
Contributor Author

lnedry commented Jan 29, 2025

I'm confident that the timeout issues have been resolved. I've also added some timeout tests.

Added tests for DNS resolver timeouts
Refactored to better handle promises and timeouts
@msimerson
Copy link
Member

msimerson commented Jan 30, 2025

Now that this PR is complete ... 😬, can I make another ask? (I'd just do it, except that then the commits would be tagged with my name, and I'd prefer you get the credit).

I just created https://github.com/haraka/haraka-plugin-mail_from.is_resolvable. You should be able to clone the repo, hop inside, and something like this should Just Work[TM]:

cd haraka-plugin-mail_from.is_resolvable
npm install --save-dev sinon
cp ~git/haraka/Haraka/plugin/mail_from.is_resolvable.js index.js
cp ~git/haraka/Haraka/test/plugins/mail_from.is_resolvable.js test/index.js
npm install
npm test

If the tests need something more feature full than node's built-in test runner, you can switch it out for mocha (test with npx mocha test).

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.

2 participants