fix: avoid double-wrapping already-wrapped errors in QueryResources

[andrest50]

Summary

• payloadToCriteria already calls wrapError internally and returns a Goa error type
• Calling wrapError again on the returned error hit the default case and invoked .Error() on the Goa type, which returns "" by design
• This produced a 500 response with {"message": ""} for any payload validation error in QueryResources
• Fix: return the already-wrapped error directly instead of double-wrapping it

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 30f95286-d656-4607-892f-c88f52a4f1f8

📥 Commits

Reviewing files that changed from the base of the PR and between 996ce22 and ee42a19.

📒 Files selected for processing (2)

• cmd/service/service.go
• cmd/service/service_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• cmd/service/service.go

---

Walkthrough

Return the original errCriteria from QueryResources when payloadToCriteria fails, rather than wrapping it; update test expectation for the pagination-invalid-token case.

Changes

Cohort / File(s)	Summary
Service logic cmd/service/service.go	QueryResources now returns errCriteria directly when payloadToCriteria fails instead of returning wrapError(ctx, errCriteria).
Tests cmd/service/service_test.go	Updated TestQuerySvcsrvc_QueryResources to expect a *querysvc.BadRequestError{} for the pagination-invalid-token case instead of *querysvc.InternalServerError{}.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: avoiding double-wrapping of errors in QueryResources, which matches the changeset's core purpose.
Description check	✅ Passed	The description is directly related to the changeset, providing clear context about the error-wrapping issue and the fix, though generated with AI assistance.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch andrest50/fix-double-wrap-empty-error-message

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

Fixes an error-handling bug in the query service’s QueryResources endpoint where errors produced by payloadToCriteria were being wrapped twice, resulting in empty error messages and incorrect 500 responses for payload/criteria parsing failures.

Changes:

• Return errCriteria from payloadToCriteria directly in QueryResources instead of calling wrapError again.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cmd/service/service.go (1)

1-174: ⚠️ Potential issue | 🟠 Major

Linting verification failed. Address the following issues before merge:

The repository has 4 linting issues preventing merge:

• pkg/httpclient/client.go:105:23: Error return value of resp.Body.Close() is not checked (errcheck)
• cmd/service/service_test.go:598:8: Type querysvc.Service should be omitted from declaration (staticcheck)
• internal/service/organization_search_test.go:763:9: Type OrganizationSearcher should be omitted from declaration (staticcheck)
• internal/service/organization_search_test.go:770:15: Type OrganizationSearcher should be omitted from declaration (staticcheck)

Note: The file under review (cmd/service/service.go) passes linting; issues are in other files within the PR.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@cmd/service/service.go` around lines 1 - 174, Handle the four lint issues:
(1) In the code that calls resp.Body.Close(), capture and check its returned
error (or defer a closure that checks and logs/returns the error) instead of
ignoring it so errcheck is satisfied; (2) In cmd/service/service_test.go remove
the explicit type annotation "querysvc.Service" from the variable declaration
(use := or var x = ...) so staticcheck no longer complains; (3) In
internal/service/organization_search_test.go at the two flagged declarations
remove the explicit "OrganizationSearcher" type from those variable declarations
(use short declaration or omit the type) so staticcheck is satisfied. Ensure you
update only the declarations/callback that match the symbols resp.Body.Close(),
querysvc.Service, and OrganizationSearcher.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@cmd/service/service.go`:
- Around line 1-174: Handle the four lint issues: (1) In the code that calls
resp.Body.Close(), capture and check its returned error (or defer a closure that
checks and logs/returns the error) instead of ignoring it so errcheck is
satisfied; (2) In cmd/service/service_test.go remove the explicit type
annotation "querysvc.Service" from the variable declaration (use := or var x =
...) so staticcheck no longer complains; (3) In
internal/service/organization_search_test.go at the two flagged declarations
remove the explicit "OrganizationSearcher" type from those variable declarations
(use short declaration or omit the type) so staticcheck is satisfied. Ensure you
update only the declarations/callback that match the symbols resp.Body.Close(),
querysvc.Service, and OrganizationSearcher.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0847b5a1-0fde-4545-be0e-2b90be5f7f7c

📥 Commits

Reviewing files that changed from the base of the PR and between 0c7ea50 and 996ce22.

📒 Files selected for processing (1)

• cmd/service/service.go