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
feat(ext/net): relevant errors for resolveDns #12370
feat(ext/net): relevant errors for resolveDns #12370
Conversation
Is it possible to write test cases which exercise some of these code paths? |
I think a |
Updated an existing test for |
I'm open but also a little hesitant as it feels like the protocol errors are very uncommon. |
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.
LGTM as this is definitely better than the current state, but I think we can do more work about this
cc @magurotuna Do you have any opinion about this?
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.
LGTM
@magurotuna, I'm merging this PR for the time being. Would you mind letting me know if you have any thoughts on this change? I'll make a PR if you suggest any changes. |
This PR maps resolveDns errors to Deno errors where applicable.
DNS protocol errors are still returned as generic errors. I'm
not sure if we should create a new DNSError class for them.
Closes #9197