Skip to content

fix(prospect): classify RQL helper errors as InvalidArgument#1692

Open
AmanGIT07 wants to merge 1 commit into
mainfrom
fix/prospect-rql-error-classification
Open

fix(prospect): classify RQL helper errors as InvalidArgument#1692
AmanGIT07 wants to merge 1 commit into
mainfrom
fix/prospect-rql-error-classification

Conversation

@AmanGIT07

Copy link
Copy Markdown
Contributor

Summary

ListProspects returned CodeInternal for every error from the service. RQL helper failures (unsupported filter/sort/group/search column from user input) now classify as CodeInvalidArgument, mirroring the existing pattern in audit_record.go.

Changes

  • core/prospect/errors.go: add ErrRepositoryBadInput sentinel.
  • internal/store/postgres/prospect_repository.go: new wrapBadInput helper; the four RQL helper error sites (AddRQLFiltersInQuery, AddRQLSearchInQuery, AddGroupInQuery, AddRQLSortInQuery) wrap with ErrRepositoryBadInput instead of the generic queryErr. ToSQL() and database execution errors keep their existing wraps.
  • internal/api/v1beta1connect/prospect.go: replace the single CodeInternal branch in ListProspects with a switch ladder: ErrInvalidUUID/ErrRepositoryBadInputCodeInvalidArgument, ErrNotExistCodeNotFound, default → CodeInternal.
  • internal/api/v1beta1connect/prospect_test.go: add cases for ErrRepositoryBadInputCodeInvalidArgument and ErrNotExistCodeNotFound; existing CodeInternal case unchanged.

Technical Details

  • Pattern matches internal/api/v1beta1connect/audit_record.go:128-135 and internal/store/postgres/audit_record_repository.go:27 (wrapValidationError). Named differently here because both files live in package postgres and Go disallows duplicate top-level identifiers.
  • Service layer (core/prospect/service.go) is a pass-through; no changes needed.

Test Plan

  • go test ./internal/api/v1beta1connect/...TestHandler_ListProspects 4/4 pass.
  • go test ./internal/store/postgres/... — Postgres integration suite green.
  • go test ./core/prospect/... — green.
  • go build ./... passes.
  • golangci-lint run on touched packages — 0 issues.

SQL Safety (if your PR touches *_repository.go or goqu.*)

  • Values flow through ? placeholders, goqu.Ex{}, or goqu.Record{} — never fmt.Sprintf or + building a query that gets executed.
  • ToSQL() callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Never query, _, err := ….
  • No ? placeholders inside single-quoted SQL literals in goqu.L (use make_interval(hours => ?)-style functions instead).
  • Any //nolint:forbidigo or // #nosec G20x annotation has a one-line justification on the same line that a reviewer can verify.

🤖 Generated with Claude Code

ListProspects returned CodeInternal for every error path. RQL helper
errors (unsupported filter/sort/group/search column) are user input
problems, not server faults — they now flow through a new
prospect.ErrRepositoryBadInput sentinel, wrapped by the repo and mapped
to connect.CodeInvalidArgument in the handler. Mirrors the audit_record
error classification ladder.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment Jun 9, 2026 10:54am

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2f9a20d0-1037-4a5c-b0c9-635034df1049

📥 Commits

Reviewing files that changed from the base of the PR and between 54e8269 and c4d67cb.

📒 Files selected for processing (4)
  • core/prospect/errors.go
  • internal/api/v1beta1connect/prospect.go
  • internal/api/v1beta1connect/prospect_test.go
  • internal/store/postgres/prospect_repository.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling in the ListProspects endpoint. The API now returns more accurate error codes: invalid input requests receive a 400-level response, missing resources receive 404, and server errors receive 500, instead of generic internal errors for all cases. This provides clearer error responses for API consumers.

Walkthrough

This PR introduces explicit error handling for the ListProspects operation. A new ErrRepositoryBadInput error is defined and used to wrap query-building errors in the repository layer, while the API handler maps service errors to appropriate Connect error codes with test coverage for the new error cases.

Changes

Error handling for ListProspects

Layer / File(s) Summary
Error contract and repository layer
core/prospect/errors.go, internal/store/postgres/prospect_repository.go
Adds ErrRepositoryBadInput error constant to the prospect package and introduces a wrapBadInput helper function. The repository's List method now wraps query-building errors (filter, search, group, sort) with wrapBadInput instead of generic fmt.Errorf formatting.
API error code mapping and tests
internal/api/v1beta1connect/prospect.go, internal/api/v1beta1connect/prospect_test.go
ListProspects handler now translates service-layer errors to Connect error codes: ErrInvalidUUID and ErrRepositoryBadInput map to CodeInvalidArgument, ErrNotExist maps to CodeNotFound, and others to CodeInternal. Tests validate error mapping for invalid input and not-found scenarios.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 27201312513

Coverage increased (+0.004%) to 43.375%

Details

  • Coverage increased (+0.004%) from the base build.
  • Patch coverage: 7 uncovered changes across 1 file (7 of 14 lines covered, 50.0%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
internal/store/postgres/prospect_repository.go 7 0 0.0%
Total (2 files) 14 7 50.0%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 37021
Covered Lines: 16058
Line Coverage: 43.38%
Coverage Strength: 12.34 hits per line

💛 - Coveralls

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.

2 participants