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

Include the DNS record received when unable to parse a CAA record #196

Conversation

rdoherty-fastly
Copy link
Contributor

This helps with debugging to see the record that DnsRuby sees. It also helps if the DNS record changed between when the exception was raised and when you view the logs. This happens a lot with our infrastructure 🙂 LMK if this is sane/useful!

@keithrbennett
Copy link
Contributor

I'm a big fan of giving more information like this. Perhaps one could do a sanity check on the length though? So change the parameter inserted into the message to something like input.to_s[0...10000] to avoid the unlikely but possible situation where the string would be huge (and not a valid input)?

@rdoherty-fastly
Copy link
Contributor Author

I'm a big fan of giving more information like this. Perhaps one could do a sanity check on the length though? So change the parameter inserted into the message to something like input.to_s[0...10000] to avoid the unlikely but possible situation where the string would be huge (and not a valid input)?

Seems like a good idea! Will update.

This helps with debugging to see the record that DnsRuby sees. It also helps if the DNS record changed between when the exception was raised and when you view the logs.
@rdoherty-fastly rdoherty-fastly force-pushed the include-input-when-throwing-error branch from 236e736 to 7c6cb2e Compare June 20, 2024 22:39
@alexdalitz alexdalitz merged commit a286841 into alexdalitz:master Jun 21, 2024
4 checks passed
@alexdalitz
Copy link
Owner

Merged - thanks!

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.

3 participants