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

Emit DNSimple API error details #199

Merged
merged 4 commits into from Nov 15, 2021
Merged

Conversation

cttttt
Copy link
Contributor

@cttttt cttttt commented Nov 12, 2021

What's the problem?

When the DNSimple API returns validation errors, although the type of the API error is included in exceptions (Validation failed), the details of the error are omitted.

For example, if we somehow attempted to put a zone into an inconsistent state (and this passed through the validations in descendants of RecordStore::Record) an exception with the following message would be emitted by record_store:

Dnsimple::RequestError: Validation failed

This doesn't give us much to go on as far as how to do these validations in RecordStore itself.

How does this fix it?

It appears as though the DNSimple API includes these details in the errors field of responses. This change simply ensures that these details are included in exceptions.

So, instead of the error above, this change would add a little bit more flavour:

Validation failed: A CNAME record exists for inconsistent-records.dns-scratch.me, cannot add another record

What do I need from a review

  • Another set of eyes for style.
  • Any situations where we'd break where error responses do not contain these extra fields. I added a test case, but feel free to suggest more testing here.

Follow up

And don't worry. I'll follow up by trying to get this change into upstream. If they accept it, we can upgrade the dependancy here, and remove the monkey-patch.

@cttttt cttttt merged commit 017128e into master Nov 15, 2021
@cttttt cttttt deleted the emit-dnsimple-api-error-details branch November 15, 2021 15:08
@shopify-shipit shopify-shipit bot temporarily deployed to production November 15, 2021 18:36 Inactive
@shopify-shipit shopify-shipit bot temporarily deployed to production July 7, 2023 18:42 Inactive
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.

None yet

2 participants