fix: skips and logs error for claims that fail to be verified

[semmet95]

No description provided.

[qodo-code-review]

Review Summary by Qodo

Skip and log errors during claim verification process

🐞 Bug fix ✨ Enhancement

Walkthroughs

Description

• Adds structured logging for claim verification failures
• Skips failed claims instead of interrupting entire verification process
• Logs errors when retrieving proofs, claims, or updating verification status
• Improves error resilience by continuing verification on individual claim failures

Diagram

flowchart LR
  A["VerifyAllClaims"] --> B["Get Proofs"]
  B -->|Error| C["Log Error"]
  C --> D["Continue"]
  A --> E["Get Claim by Digest"]
  E -->|Error| F["Log Error"]
  F --> D
  A --> G["Update Claims"]
  G -->|Error| H["Log Error"]
  H --> I["Return Error"]

Loading

File Changes

1. pkg/domain/claim/claim_service.go Error handling +5/-1

Add error logging and skip failed claims

• Imports log/slog for structured logging
• Adds error logging when retrieving proofs map fails
• Changes claim retrieval error handling from returning to logging and continuing
• Adds error logging when updating claims for verification fails

pkg/domain/claim/claim_service.go

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1)

1. slog.Error uses fmt.Sprintf 📘 Rule violation ◔ Observability

Description

The new log statement interpolates claim into the log message via fmt.Sprintf, producing an
unstructured message and embedding potentially untrusted data directly in the message string. This
weakens structured JSON logging and increases risk of log injection/poor searchability compared to
logging claim as a dedicated structured field.

Code

pkg/domain/claim/claim_service.go[R171-172]

+			slog.Error(fmt.Sprintf("failed to get claim for digest: %s", claim), "error", err)
+			continue

Evidence

The checklist requires structured JSON logging and avoiding ad-hoc string formatting in log
messages; the added line formats the message with fmt.Sprintf("...%s", claim) rather than logging
claim as a structured attribute. The checklist also calls out using raw external/user input in log
messages without sanitization as a red flag; claim is inserted directly into the message string.

Rule 467919: Use structured JSON logging in all services
Rule 467901: Sanitize external and user input before use
pkg/domain/claim/claim_service.go[171-172]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new error log message is built with `fmt.Sprintf(...)` and embeds `claim` directly into the log message string, instead of keeping the message constant and logging `claim` as a structured attribute.

## Issue Context
The service is using `log/slog` with JSON logging, so values like the claim digest should be emitted as key/value fields for consistent structure and safer handling of untrusted strings.

## Fix Focus Areas
- pkg/domain/claim/claim_service.go[171-172]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Skipped claim lookup errors 🐞 Bug ☼ Reliability

Description

VerifyAllClaims continues on any GetClaimByUriDigest error and can return nil even when some claims
failed to load, causing silent partial verification. This masks transient DB/infra failures from the
caller (the handler only logs failure when VerifyAllClaims returns a non-nil error).

Code

pkg/domain/claim/claim_service.go[R169-173]

		updatedClaim, err := svc.GetClaimByUriDigest(ctx, claim)
		if err != nil {
-			return err
+			slog.Error(fmt.Sprintf("failed to get claim for digest: %s", claim), "error", err)
+			continue
		}

Evidence

VerifyAllClaims now logs and continues instead of returning when GetClaimByUriDigest fails, so the
job can complete and update only a subset of claims. The HTTP handler treats a nil return as success
(no failure log), so these partial failures become silent. GetClaimByUriDigest only wraps
record-not-found into ErrClaimNotFound; all other errors (e.g., DB connectivity) would also be
skipped under the current logic.

pkg/domain/claim/claim_service.go[146-194]
pkg/domain/claim/claim_service.go[60-69]
pkg/handlers/claim.go[194-205]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`VerifyAllClaims` currently `continue`s on *any* error returned by `GetClaimByUriDigest`, which can hide transient failures and lead to partial verification while still returning `nil`.

### Issue Context
The caller (`ClaimHandler.VerifyAllClaims`) only logs job failure when `VerifyAllClaims` returns a non-nil error, so swallowed errors won’t be surfaced.

### Fix Focus Areas
- pkg/domain/claim/claim_service.go[146-194]
- pkg/domain/claim/claim_service.go[60-69]
- pkg/handlers/claim.go[194-205]

### What to change
- In the `GetClaimByUriDigest` error branch inside `VerifyAllClaims`, only `continue` for `errors.Is(err, apperrors.ErrClaimNotFound)` (or underlying `gorm.ErrRecordNotFound`), since that represents a dangling proof/claim reference.
- For any other error, return the error (fail the job) or aggregate and return a final error indicating partial failure.
- (Optional but recommended) Add/adjust unit tests to cover:
 - `ErrClaimNotFound` is skipped.
 - Non-notfound errors cause `VerifyAllClaims` to return an error.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Contextless error logging 🐞 Bug ◔ Observability

Description

New error logs in VerifyAllClaims use slog.Error instead of slog.ErrorContext(ctx,...), so
context-injected log attributes (e.g., request headers) won’t be attached. This reduces traceability
of verification job failures and makes production debugging harder.

Code

pkg/domain/claim/claim_service.go[R147-151]

	claimsProofs, err := svc.proofSvc.GetProofsByClaims(ctx)
	if err != nil {
+		slog.Error("failed to get proofs to claims map", "error", err)
		return err
	}

Evidence

The app installs a slog handler that enriches records with attributes stored in the context, and
middleware appends request header fields into the gin context. Using slog.Error (without context)
bypasses that enrichment, whereas other parts of the codebase already use *Context logging (e.g.,
slog.InfoContext in repositories).

pkg/domain/claim/claim_service.go[146-191]
cmd/app/main.go[32-49]
pkg/logger/logger.go[14-27]
pkg/domain/claim/claim_repository.go[36-47]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`VerifyAllClaims` uses `slog.Error(...)` for newly added logs. This drops the request/job context fields appended via `logger.ContextHandler`.

### Issue Context
The default slog logger is configured with `logger.ContextHandler`, and middleware appends request header attributes into the gin context; these only show up when logging via `*Context` methods.

### Fix Focus Areas
- pkg/domain/claim/claim_service.go[146-191]
- cmd/app/main.go[32-49]
- pkg/logger/logger.go[14-27]

### What to change
- Replace the new `slog.Error(...)` calls with `slog.ErrorContext(ctx, ...)`.
- Prefer structured attributes instead of `fmt.Sprintf` in the message, e.g.:
 - `slog.ErrorContext(ctx, "failed to get claim", "uriDigest", claim, "error", err)`
- Keep message strings stable and put dynamic values into key/value fields for easier filtering/searching.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[cubic-dev-ai]

No issues found across 1 file