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

test: set dns-resolveany-bad-ancount timeout #50995

Closed
wants to merge 3 commits into from

Conversation

kapouer
Copy link
Contributor

@kapouer kapouer commented Dec 1, 2023

Some build environments just hang forever.

Some build environments just hang forever
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Dec 1, 2023
@lpinca lpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 1, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 1, 2023
@nodejs-github-bot
Copy link
Collaborator

@kapouer
Copy link
Contributor Author

kapouer commented Dec 2, 2023

Those changes don't make any sense, the server should respond right away in that test.
Something else is happening, closing.

@kapouer kapouer closed this Dec 2, 2023
@lpinca
Copy link
Member

lpinca commented Dec 3, 2023

@kapouer they does, malformed responses are discarded in c-ares >= 1.21.0. See #50741. The same issue is blocking #50800. I think it makes sense to completely remove the test.

@kapouer
Copy link
Contributor Author

kapouer commented Dec 3, 2023

Yes, but setting the timeout as in this PR does not work at all.

@lpinca
Copy link
Member

lpinca commented Dec 3, 2023

The only problem I actually see in this PR is the retry option name. It is called tries. I think that if you use the correct name it works.

You might also wonder why setting a timeout is required. The default one is two seconds. The problem is that after c-ares/c-ares@3b10e57 using -1 for the default timeout no longer works. As per our documentation we pass -1 but that is casted to an unsigned int (4294967295). This is an issue we should address before updating c-ares to version >= 1.21.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants