-
-
Notifications
You must be signed in to change notification settings - Fork 21k
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
Prevent HTTPRequest from polling invalid client #64472
Conversation
3826355
to
63378c2
Compare
361a0d2
to
a8906f5
Compare
Reworked this PR, somewhat. Now it returns early, and the HTTPRequest... request isn't even started if the ID is invalid. |
Could you squash the commits? See PR workflow for instructions. |
As I do that, what would the most appropriate error be for this? |
a8906f5
to
9d0cf58
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for looking into this issue, see my review comments for changes.
9d0cf58
to
e5304b2
Compare
e5304b2
to
9e284f6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, thanks! 🏆
Cherry-picked for 3.6. |
Cherry-picked for 3.5.1. |
Somewhat fixes #60602
The reason why the error mentioned in the issue typically shows up in the first place is because HTTPRequest keeps polling its associated client. However, as a result of exceeding the maximum DNS requests, the client's resolving id is INVALID, but nothing seems to be done on the HTTPRequest's side to handle it, because the HTTPRequest doesn't know this happened.
Now, only this error show up (no edit on my part, this is appropriate):
HTTPClientTCP.
connect_to_host()
now returns ERR_CANT_RESOLVE if the maximum number of DNS queries has been reached,and so does HTTPRequest.request()
.Need to update documentation, but I know ERR_BUSY isn't the most ideal error code (it's already used, too). I need someone to suggest an alternative.