HYPERFLEET-1247 - feat: introduce NTF-000 error code for endpoint not found#251
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (4)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
Risk Score: 0 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 41 lines | +0 |
| Sensitive paths | none | +0 |
| Test coverage | Tests cover changed packages | +0 |
Computed by hyperfleet-risk-scorer
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/api/error.go (1)
22-30: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAlign 404 text fields with
CodeNotFoundEndpointsemantics (CWE-436).
Codenow indicates endpoint-not-found, butTitle/Detailstill describe resource-not-found. Keep these fields consistent so clients and operators don’t misclassify route misses.Suggested patch
- detail := fmt.Sprintf("The requested resource '%s' doesn't exist", r.URL.Path) + detail := fmt.Sprintf("The requested endpoint '%s' does not exist", r.URL.Path) ... - Title: "Resource Not Found", + Title: "Endpoint Not Found",🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/api/error.go` around lines 22 - 30, The 404 ProblemDetails in error handling are inconsistent with CodeNotFoundEndpoint semantics because the Title and Detail still describe a generic resource miss. Update the response construction in the error response path that builds openapi.ProblemDetails so the Title and Detail consistently describe an endpoint/route not found, matching the CodeNotFoundEndpoint value and the existing errors.ErrorTypeNotFound. Keep the same identifiers and structure, just align the text fields with the endpoint-not-found meaning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/errors/errors.go`:
- Around line 43-44: The route-level 404 contract is using the wrong shared code
in docs and tests, while `CodeNotFoundEndpoint` should be the published value
for endpoint 404s and `CodeNotFoundGeneric` should remain for resource lookup
failures. Update every place that publishes or asserts the route-level 404
response to use `CodeNotFoundEndpoint` consistently, and leave
`CodeNotFoundGeneric` unchanged for generic/not-found resource cases; use the
`pkg/errors` constants as the source of truth when fixing downstream fixtures
and documentation.
---
Outside diff comments:
In `@pkg/api/error.go`:
- Around line 22-30: The 404 ProblemDetails in error handling are inconsistent
with CodeNotFoundEndpoint semantics because the Title and Detail still describe
a generic resource miss. Update the response construction in the error response
path that builds openapi.ProblemDetails so the Title and Detail consistently
describe an endpoint/route not found, matching the CodeNotFoundEndpoint value
and the existing errors.ErrorTypeNotFound. Keep the same identifiers and
structure, just align the text fields with the endpoint-not-found meaning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: c06c4e39-beaa-40d9-800b-c418a45f5bc3
📒 Files selected for processing (2)
pkg/api/error.gopkg/errors/errors.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
… found The catch-all 404 handler now returns HYPERFLEET-NTF-000 instead of NTF-001, allowing consumers to distinguish between a broken/misconfigured URL (no route matched) and a genuine resource not found (force-delete).
f5a4809 to
8e6c795
Compare
Summary
CodeNotFoundEndpoint(HYPERFLEET-NTF-000) for the catch-all 404 handler (no route matched)SendNotFound()now returnsNTF-000instead ofNTF-001, allowing consumers to distinguish between a broken/misconfigured URL and a genuine resource not found (force-delete)NTF-001(CodeNotFoundGeneric) remains unchanged for service-layer resource-not-found responsesTest plan
make lint— 0 issuesmake test— 1265 tests passingFixes: https://redhat.atlassian.net/browse/HYPERFLEET-1247