Apply suggestions from code review

Imorting suggested changes. More fixes to come to address remaining comments.

Co-authored-by: Quentin McGaw <[email protected]>

docs/porkbun.md

@@ -42,21 +42,11 @@
 
 In case you don't have an A or AAAA record for your host and domain combination, it will be created by DDNS-Updater.
 
-## Conflicting default records
+Porkbun creates default DNS entries for new domains, which can conflict with creating a root or wildcard A/AAAA record. Therefore, ddns-updater automatically removes any conflicting default record before creating records, as described in the table below:
 
-Porkbun sets the following default DNS entries for new domains:
+| Record type | Owner | Record value | Situation requiring a removal |
+| --- | --- | --- | --- |
+| `ALIAS` | `@` | [pixie.porkbun.com](https://pixie.porkbun.com)  | Creating A or AAAA record for the root domain **or** wildcard domain |
+| `CNAME` | `*` | [pixie.porkbun.com](https://pixie.porkbun.com)  | Creating A or AAAA record for the wildcard domain |
 
-- ALIAS `domain.tld` -> `pixie.porkbun.com`
-- CNAME `*.domain.tld` -> `pixie.porkbun.com`
-
-`pixie.porkbun.com` is porkbun's Parked domain website.
-
-[Parked domain screenshot](https://github.com/user-attachments/assets/d73c4fbd-f6a9-48c9-9dcb-01818541ceb1)
-
-In-order to create an A or AAAA DNS record, when setting the `domain.tld` or `*.domain.tld`, if a respective ALIAS or CNAME record is already set to `pixie.porkbun.com`, the respective default record will be automatically deleted by DDNS-Updater and a new A or AAAA record will be created.
 More details is in [this comment by @everydaycombat](https://github.com/qdm12/ddns-updater/issues/546#issuecomment-1773960193).
-
->NOTE: DDNS-Updater will only attempt to delete the default records for `domain.tld` and `*.domain.tld`.
->
->If there are any other conflicting records set for other domains, DDNS-Updater will not check for them, and it will simply attempt to create an `A` or `AAAA` record.
->Porkbun's API error messages are extremely unhelpful (`400: something went wrong`), so it is recommended to check the domain record from the Porkbun Web-UI in the event of unexpected 400 API errors to ensure there are no conflicting records present.

internal/provider/providers/porkbun/api.go

@@ -12,7 +12,6 @@ import (
 	"github.com/qdm12/ddns-updater/internal/provider/errors"
 )
 
-// Leaving unused values commented out to improve resilience to API changes.
 type dnsRecord struct {
 	ID   string `json:"id"`
 	Name string `json:"name"`

internal/provider/providers/porkbun/provider.go

@@ -136,7 +136,7 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip netip.Add
 			// Delete ALIAS domain.tld -> pixie.porkbun.com record
 			err = p.deleteMatchingRecord(ctx, client, constants.ALIAS, porkbunParkedDomain)
 			if err != nil {
-				return netip.Addr{}, err
+				return netip.Addr{}, fmt.Errorf("deleting default parked domain record: %w", err)
 			}
 		case p.owner == "*":
 			// Delete CNAME *.domain.tld -> pixie.porkbun.com record
@@ -163,31 +163,29 @@ func (p *Provider) Update(ctx context.Context, client *http.Client, ip netip.Add
 	return ip, nil
 }
 
-// Deletes the matching record for a specific recordType iff the content matches the expected content value.
-// Returns an error if there is a matching record but an unexpected content value.
-// If there is no matching record, there is nothing to do.
-func (p *Provider) deleteMatchingRecord(
-	ctx context.Context,
-	client *http.Client,
-	recordType string,
-	expectedContent string,
-) error {
+// deleteMatchingRecord deletes an eventually present record matching a specific record type if the content matches the expected content value.
+// It returns an error if multiple records are found or if one record is found with an unexpected value.
+func (p *Provider) deleteMatchingRecord(ctx context.Context, client *http.Client, 
+    recordType, expectedContent string) (err error) {
 	records, err := p.getRecords(ctx, client, recordType)
 	if err != nil {
 		return fmt.Errorf("getting %s records: %w", recordType, err)
 	}
 
-	if len(records) == 1 && records[0].Content == expectedContent {
-		err = p.deleteRecord(ctx, client, recordType)
-		if err != nil {
-			return fmt.Errorf("deleting %s record: %w", recordType, err)
-		}
+	switch {
+	case len(records) == 0:
 		return nil
-	} else if len(records) >= 1 {
-		// We have 1 or more matching records, but the expectedContent didn't match. Throw a conflicting record error.
-		return fmt.Errorf("deleting %s record: %w", recordType, errors.ErrConflictingRecord)
+	case len(records) > 1:
+		return fmt.Errorf("%w: %d %s records are already set", errors.ErrConflictingRecord, recordType)
+	case records[0].Content != expectedContent:
+		return fmt.Errorf("%w: %s record has content %q mismatching expected content %q",
+			errors.ErrConflictingRecord, recordType, records[0].Content, expectedContent)
+	}
+
+	// Single record with content matching expected content.
+	err = p.deleteRecord(ctx, client, recordType)
+	if err != nil {
+		return fmt.Errorf("deleting record: %w", err)
 	}
-
-	// No record found for recordType, nothing to do.
 	return nil
 }