fix!: replaced sdkerrors.Register with sdkerrors.RegisterWithGRPCCode for the proper error propagation. #173
Conversation
WalkthroughReplace many package error initializers with gRPC-aware registrations by switching to sdkerrors.RegisterWithGRPCCode(...) and importing google.golang.org/grpc/codes; add table-driven tests that assert each module error maps to the expected gRPC status code. (≤50 words) Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
go/node/deployment/v1/errors.go
Outdated
| ErrInvalidDeployment = cerrors.RegisterWithGRPCCode(ModuleName, errInvalidDeployment, codes.InvalidArgument, "Invalid deployment") | ||
| // ErrInvalidGroupID is the error when already deployment exists | ||
| ErrInvalidGroupID = cerrors.Register(ModuleName, errInvalidGroupID, "Deployment exists") | ||
| ErrInvalidGroupID = cerrors.RegisterWithGRPCCode(ModuleName, errInvalidGroupID, codes.InvalidArgument, "Deployment exists") |
There was a problem hiding this comment.
I am not sure about this error mapping - it can be both codes.InvalidArgument and codes.AlreadyExists.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
go/node/deployment/v1/errors.gogo/node/deployment/v1/errors_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
go/node/deployment/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(17-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: go
🔇 Additional comments (5)
go/node/deployment/v1/errors.go (3)
5-5: LGTM! Correct import for gRPC codes.The addition of the
google.golang.org/grpc/codesimport is necessary for the newRegisterWithGRPCCodecalls and enables proper HTTP status code mapping.
36-42: Excellent fix for the core issue!The mapping of
ErrDeploymentNotFoundtocodes.NotFound(line 42) directly addresses issue #404 by ensuring that missing deployment queries return HTTP 404 instead of HTTP 500. The gRPC code mappings for related "not found" errors are also correct.
44-81: LGTM! All remaining gRPC code mappings are semantically correct.The error-to-status-code mappings are appropriate:
FailedPreconditionfor state-based errors (closed, open, paused deployments/groups)InvalidArgumentfor all validation failuresInternalfor internal server errorsThis will ensure correct HTTP status codes are returned via grpc-gateway.
go/node/deployment/v1/errors_test.go (2)
1-11: LGTM! Proper test structure and imports.Using an external test package (
v1_test) is good practice for testing public APIs, and the imports are correct for validating gRPC status code mappings.
13-143: Excellent test coverage and implementation!The test provides complete coverage of all 23 error definitions with correct validation approach:
- Uses
status.FromErrorto properly extract gRPC status codes- Verifies both conversion success and code correctness
- Table-driven structure with descriptive test case names
- Covers all gRPC code categories: NotFound (3), AlreadyExists (1), InvalidArgument (13), FailedPrecondition (5), and Internal (1)
This ensures the fix for issue #404 is properly validated and prevents regressions.
…to propagate the proper http code into the response
vertex451
force-pushed
the
artem/fix/500-instead-of-404
branch
from
e970797 to
dc80e0d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go/node/deployment/v1/errors_test.go (1)
13-13: Consider adding a package-level doc comment.A brief comment explaining that this test ensures proper HTTP status codes via grpc-gateway mapping would help future maintainers understand the test's purpose in the context of the HTTP 500 → 404 fix.
🔎 Suggested addition
+// Package v1_test validates gRPC status code mappings for deployment errors. +// These mappings ensure grpc-gateway returns correct HTTP status codes +// (e.g., 404 for NotFound instead of 500 for Unknown). + package v1_test
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
go/node/deployment/v1/errors.gogo/node/deployment/v1/errors_test.go
🧰 Additional context used
🧬 Code graph analysis (2)
go/node/deployment/v1/errors_test.go (1)
go/node/deployment/v1/errors.go (23)
ErrDeploymentNotFound(42-42)ErrGroupNotFound(62-62)ErrNameDoesNotExist(36-36)ErrDeploymentExists(40-40)ErrInvalidRequest(38-38)ErrInvalidGroups(48-48)ErrInvalidDeploymentID(50-50)ErrEmptyHash(52-52)ErrInvalidHash(54-54)ErrInvalidDeployment(58-58)ErrInvalidGroupID(60-60)ErrInvalidDeposit(74-74)ErrInvalidIDPath(76-76)ErrInvalidParam(78-78)ErrInvalidEscrowID(80-80)ErrOwnerAcctMissing(46-46)ErrGroupSpecInvalid(72-72)ErrDeploymentClosed(44-44)ErrGroupClosed(64-64)ErrGroupOpen(66-66)ErrGroupPaused(68-68)ErrGroupNotOpen(70-70)ErrInternal(56-56)
go/node/deployment/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(17-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
- GitHub Check: proto
🔇 Additional comments (2)
go/node/deployment/v1/errors.go (1)
5-5: LGTM! Error-to-HTTP-status mappings are correct.The addition of explicit gRPC codes to all error registrations directly addresses the PR objective. The NotFound codes (lines 36, 42, 62) will now correctly map to HTTP 404 instead of 500 via grpc-gateway, fixing the reported issue. All other code mappings are semantically appropriate:
AlreadyExists→ HTTP 409InvalidArgument→ HTTP 400FailedPrecondition→ HTTP 400Internal→ HTTP 500Also applies to: 36-80
go/node/deployment/v1/errors_test.go (1)
13-143: LGTM! Comprehensive test coverage for gRPC code mappings.The test validates all 23 error variables against their expected gRPC codes using
status.FromError, which confirms the error registrations will produce the correct HTTP status codes via grpc-gateway. Test structure is clean and follows Go's table-driven testing pattern.
…in audit, cert, escrow, market, provider, take and types modules
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
go/node/market/v1/errors.go (1)
111-112: Add documentation comment forErrInvalidEscrowID.All other errors in this file have documentation comments. For consistency, add one for the new error.
🔎 Suggested fix
// ErrInvalidLeaseClosedReason indicates reason for lease close does not match context ErrInvalidLeaseClosedReason = sdkerrors.RegisterWithGRPCCode(ModuleName, errInvalidLeaseClosedReason, codes.InvalidArgument, "invalid lease closed reason") - ErrInvalidEscrowID = sdkerrors.RegisterWithGRPCCode(ModuleName, errInvalidEscrowID, codes.InvalidArgument, "invalid escrow id") + // ErrInvalidEscrowID indicates an invalid escrow ID + ErrInvalidEscrowID = sdkerrors.RegisterWithGRPCCode(ModuleName, errInvalidEscrowID, codes.InvalidArgument, "invalid escrow id")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
go/node/audit/v1/errors.gogo/node/audit/v1/errors_test.gogo/node/cert/v1/errors.gogo/node/cert/v1/errors_test.gogo/node/deployment/v1/errors.gogo/node/escrow/module/error.gogo/node/escrow/module/error_test.gogo/node/market/v1/errors.gogo/node/market/v1/errors_test.gogo/node/provider/v1beta4/errors.gogo/node/provider/v1beta4/errors_test.gogo/node/take/v1/errors.gogo/node/take/v1/errors_test.gogo/node/types/attributes/v1/attribute.gogo/node/types/attributes/v1/errors_test.gogo/node/types/deposit/v1/errors.gogo/node/types/deposit/v1/errors_test.go
🧰 Additional context used
🧬 Code graph analysis (16)
go/node/take/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/audit/v1/errors_test.go (1)
go/node/audit/v1/errors.go (3)
ErrProviderNotFound(16-16)ErrAttributeNotFound(22-22)ErrInvalidAddress(19-19)
go/node/take/v1/errors_test.go (4)
go/node/market/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-198)go/node/types/attributes/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-38)go/node/types/deposit/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-38)go/node/take/v1/errors.go (1)
ErrInvalidParam(14-14)
go/node/escrow/module/error_test.go (1)
go/node/escrow/module/error.go (15)
ErrAccountExists(29-29)ErrPaymentExists(34-34)ErrInvalidDenomination(33-33)ErrPaymentRateZero(37-37)ErrInvalidPayment(38-38)ErrInvalidSettlement(39-39)ErrInvalidID(40-40)ErrInvalidAccount(41-41)ErrInvalidAccountDepositor(42-42)ErrInvalidDeposit(44-44)ErrInvalidAuthzScope(45-45)ErrAccountClosed(30-30)ErrAccountOverdrawn(32-32)ErrPaymentClosed(35-35)ErrUnauthorizedDepositScope(43-43)
go/node/cert/v1/errors_test.go (3)
go/node/market/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-198)go/node/take/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-33)go/node/cert/v1/errors.go (9)
ErrCertificateNotFound(28-28)ErrCertificateExists(34-34)ErrInvalidAddress(31-31)ErrInvalidSerialNumber(40-40)ErrInvalidCertificateValue(43-43)ErrInvalidPubkeyValue(46-46)ErrInvalidState(49-49)ErrInvalidKeySize(52-52)ErrCertificateAlreadyRevoked(37-37)
go/node/audit/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/market/v1/errors_test.go (1)
go/node/market/v1/errors.go (34)
ErrUnknownBid(59-59)ErrUnknownLease(61-61)ErrUnknownLeaseForBid(63-63)ErrUnknownOrderForBid(65-65)ErrNoLeaseForOrder(73-73)ErrOrderNotFound(75-75)ErrGroupNotFound(77-77)ErrBidNotFound(83-83)ErrLeaseNotFound(87-87)ErrUnknownProvider(107-107)ErrBidExists(89-89)ErrOrderExists(97-97)ErrEmptyProvider(47-47)ErrSameAccount(49-49)ErrBidOverOrder(53-53)ErrAttributeMismatch(55-55)ErrCapabilitiesMismatch(57-57)ErrBidZeroPrice(85-85)ErrBidInvalidPrice(91-91)ErrInvalidDeposit(103-103)ErrInvalidParam(105-105)ErrInvalidBid(109-109)ErrInvalidLeaseClosedReason(111-111)ErrInvalidEscrowID(112-112)ErrLeaseNotActive(67-67)ErrBidNotActive(69-69)ErrBidNotOpen(71-71)ErrGroupNotOpen(79-79)ErrOrderNotOpen(81-81)ErrOrderActive(93-93)ErrOrderClosed(95-95)ErrOrderTooEarly(99-99)ErrOrderDurationExceeded(101-101)ErrInternal(51-51)
go/node/types/deposit/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/cert/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/types/deposit/v1/errors_test.go (3)
go/node/take/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-33)go/node/types/attributes/v1/errors_test.go (1)
TestErrorGRPCStatusCodes(13-38)go/node/types/deposit/v1/errors.go (2)
ErrInvalidDepositor(17-17)ErrInvalidDepositSource(19-19)
go/node/types/attributes/v1/errors_test.go (1)
go/node/types/attributes/v1/attribute.go (2)
ErrAttributesDuplicateKeys(30-30)ErrInvalidAttributeKey(31-31)
go/node/provider/v1beta4/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/escrow/module/error.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/market/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/deployment/v1/errors.go (1)
go/node/types/attributes/v1/attribute.go (1)
ModuleName(18-18)
go/node/provider/v1beta4/errors_test.go (1)
go/node/provider/v1beta4/errors.go (8)
ErrProviderNotFound(27-27)ErrProviderExists(30-30)ErrInvalidProviderURI(21-21)ErrNotAbsProviderURI(24-24)ErrInvalidAddress(33-33)ErrAttributes(36-36)ErrInvalidInfoWebsite(42-42)ErrIncompatibleAttributes(39-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: test
- GitHub Check: coverage
- GitHub Check: go
- GitHub Check: proto
🔇 Additional comments (24)
go/node/audit/v1/errors.go (1)
14-22: LGTM! gRPC code mappings are semantically correct.The error-to-gRPC-code assignments align well with standard conventions:
ErrProviderNotFoundandErrAttributeNotFound→codes.NotFoundErrInvalidAddress→codes.InvalidArgumentThis correctly addresses the HTTP 500 → 404 issue for "not found" semantics.
go/node/provider/v1beta4/errors.go (1)
19-43: LGTM! Comprehensive and semantically correct gRPC code mappings.The error registrations follow gRPC conventions properly:
- Validation errors (
ErrInvalidProviderURI,ErrNotAbsProviderURI,ErrInvalidAddress, etc.) →codes.InvalidArgument- Resource not found (
ErrProviderNotFound) →codes.NotFound- Duplicate resource (
ErrProviderExists) →codes.AlreadyExists- State violation (
ErrIncompatibleAttributes) →codes.FailedPreconditionThis ensures proper HTTP status code mapping via grpc-gateway.
go/node/deployment/v1/errors.go (2)
41-42: Key fix for issue #404: ErrDeploymentNotFound now maps to HTTP 404.This is the core change that resolves the bug. With
codes.NotFound, the grpc-gateway will correctly translate this to HTTP 404 instead of the previous HTTP 500.
34-81: Comprehensive gRPC code mappings look correct.All error-to-code mappings follow gRPC semantic conventions:
NotFoundfor missing resources (deployment, group, name)AlreadyExistsfor duplicate resourcesFailedPreconditionfor state violations (closed, paused, open states)InvalidArgumentfor validation errorsInternalfor internal errorsThe previous concern about
ErrInvalidGroupID(lines 59-60) has been addressed with the corrected comment and message.go/node/take/v1/errors.go (1)
12-15: LGTM!The
ErrInvalidParamcorrectly maps tocodes.InvalidArgument, which aligns with the semantic meaning of an invalid parameter.go/node/types/attributes/v1/errors_test.go (1)
13-38: LGTM! Well-structured test for gRPC status code mappings.The table-driven test pattern with subtests is clean and maintainable. The test correctly validates that:
- Errors are convertible to gRPC status via
status.FromError- The resulting status codes match expectations
This provides good regression protection for the error-to-HTTP-status mapping behavior.
go/node/take/v1/errors_test.go (1)
13-33: LGTM!Consistent test structure with other error test files. Validates the
ErrInvalidParam→codes.InvalidArgumentmapping.go/node/audit/v1/errors_test.go (1)
13-43: LGTM! Complete test coverage for audit/v1 errors.All three errors defined in
errors.goare covered with correct expected codes. The test structure is consistent with other modules in this PR.go/node/escrow/module/error_test.go (1)
13-113: LGTM! Comprehensive test coverage for escrow module errors.Excellent coverage of 16 error types across 5 different gRPC status codes (
NotFound,AlreadyExists,InvalidArgument,FailedPrecondition,PermissionDenied). The test cases align with the production error definitions inerror.go.go/node/market/v1/errors_test.go (1)
1-198: Comprehensive test coverage for market error mappings.The test correctly validates that all market module errors map to their intended gRPC status codes. The table-driven approach is clear and maintains consistency with error definitions in
errors.go.go/node/types/deposit/v1/errors_test.go (1)
1-38: Test coverage validated for deposit errors.The test correctly verifies that deposit-related errors map to
codes.InvalidArgument, aligning with the updated error registrations.go/node/cert/v1/errors.go (2)
7-7: Import added for gRPC codes.The addition of
google.golang.org/grpc/codesis necessary for the RegisterWithGRPCCode calls.
28-52: gRPC status codes correctly assigned to certificate errors.The error-to-code mappings are semantically appropriate:
NotFoundfor missing certificatesInvalidArgumentfor validation failuresAlreadyExistsfor duplicatesFailedPreconditionfor revoked certificatesThese changes will ensure certificate errors map to correct HTTP status codes (e.g., 404, 400, 409, 412) via gRPC-gateway.
go/node/provider/v1beta4/errors_test.go (1)
1-68: Provider error mappings validated.The test ensures provider module errors correctly convert to gRPC status codes, with appropriate mappings for not found, already exists, invalid input, and precondition failures.
go/node/cert/v1/errors_test.go (1)
1-73: Certificate error test coverage is complete.All certificate module errors are tested with correct gRPC code expectations, ensuring the changes in
errors.goare properly validated.go/node/types/deposit/v1/errors.go (2)
4-5: Imports updated for gRPC-aware error registration.The addition of
sdkerrorsalias andgrpc/codesenables explicit gRPC status code assignment.
17-19: Deposit errors correctly mapped to InvalidArgument.Both
ErrInvalidDepositorandErrInvalidDepositSourceappropriately map tocodes.InvalidArgument, which will translate to HTTP 400 for invalid request parameters.go/node/types/attributes/v1/attribute.go (2)
13-14: Imports added for gRPC code support.The sdkerrors alias and grpc/codes import enable explicit gRPC status code registration.
30-31: Attribute validation errors correctly mapped.Both attribute errors map to
codes.InvalidArgument, which is appropriate for validation failures (duplicate keys, invalid key format). These will correctly translate to HTTP 400.go/node/escrow/module/error.go (2)
4-5: Imports updated for gRPC-aware error handling.The switch to
sdkerrorsand addition ofgrpc/codesenables explicit status code mapping for escrow module errors.
29-45: Escrow error mappings are semantically correct.All 16 error registrations use appropriate gRPC codes:
NotFoundfor missing accounts/payments (→ HTTP 404)AlreadyExistsfor duplicates (→ HTTP 409)InvalidArgumentfor validation failures (→ HTTP 400)FailedPreconditionfor state violations (→ HTTP 412)PermissionDeniedfor authorization failures (→ HTTP 403)The use of
PermissionDeniedforErrUnauthorizedDepositScopeis particularly appropriate, as it distinguishes authorization failures from general validation errors.go/node/market/v1/errors.go (3)
3-6: LGTM!The import of
google.golang.org/grpc/codesis correctly added to support the gRPC code mappings.
41-43: LGTM!New error codes are correctly appended at the end of the iota sequence, preserving existing error code values for backward compatibility.
45-107: gRPC code mappings are semantically correct.The mappings appropriately use:
codes.NotFoundfor missing resources (bids, leases, orders, providers)codes.InvalidArgumentfor validation errorscodes.AlreadyExistsfor duplicate entriescodes.FailedPreconditionfor state-related errorscodes.Internalfor internal errorsThis correctly addresses the PR objective of returning HTTP 404 for not-found scenarios instead of HTTP 500.
vertex451
changed the title
vertex451
changed the title
|
this is consensus breaking change, will hold on merging for now |
…fix/500-instead-of-404
There was a problem hiding this comment.
Actionable comments posted: 2
Fix all issues with AI Agents 🤖
In @go/node/market/v1/errors.go:
- Line 77: The ErrGroupNotFound constant incorrectly registers the message
"order not found"; update the registration call for ErrGroupNotFound (using
sdkerrors.RegisterWithGRPCCode, errCodeGroupNotFound, and codes.NotFound) to use
the correct message "group not found" and, if present, update any surrounding
comment to match the "group" semantics so the name, message, and comment are
consistent.
- Line 63: ErrUnknownLeaseForBid is registered with errCodeUnknownLeaseForOrder
which mismatches the variable name and message; either add a new error code
constant named errCodeUnknownLeaseForBid and use it in
sdkerrors.RegisterWithGRPCCode for ErrUnknownLeaseForBid, or if the intent was
"order" rename ErrUnknownLeaseForBid to ErrUnknownLeaseForOrder (and update any
references) so the constant errCodeUnknownLeaseForOrder matches the error
variable and message.
🧹 Nitpick comments (1)
go/node/market/v1/errors.go (1)
112-112: Add comment for consistency with other error declarations.All other error variables have a preceding comment line, but
ErrInvalidEscrowIDis declared inline. For consistency and better documentation, add a comment on a separate line.🔎 Suggested fix
// ErrInvalidLeaseClosedReason indicates reason for lease close does not match context ErrInvalidLeaseClosedReason = sdkerrors.RegisterWithGRPCCode(ModuleName, errInvalidLeaseClosedReason, codes.InvalidArgument, "invalid lease closed reason") - ErrInvalidEscrowID = sdkerrors.RegisterWithGRPCCode(ModuleName, errInvalidEscrowID, codes.InvalidArgument, "invalid escrow id") + // ErrInvalidEscrowID indicates an invalid escrow identifier + ErrInvalidEscrowID = sdkerrors.RegisterWithGRPCCode(ModuleName, errInvalidEscrowID, codes.InvalidArgument, "invalid escrow id")
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
go/node/market/v1/errors.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: coverage
- GitHub Check: test
- GitHub Check: go
🔇 Additional comments (2)
go/node/market/v1/errors.go (2)
5-5: LGTM! Import necessary for gRPC code constants.The addition of
google.golang.org/grpc/codesis required for theRegisterWithGRPCCodecalls throughout the file.
47-112: LGTM! gRPC code mappings are semantically correct.All error registrations use appropriate gRPC codes that align with gRPC best practices:
codes.InvalidArgumentfor validation and parameter errorscodes.NotFoundfor missing resourcescodes.FailedPreconditionfor state precondition failurescodes.AlreadyExistsfor duplicate resourcescodes.Internalfor internal errorsThis ensures proper HTTP status code mapping via grpc-gateway (e.g., NotFound → HTTP 404 instead of HTTP 500).
| ErrUnknownLease = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeUnknownLease, codes.NotFound, "unknown lease") | ||
| // ErrUnknownLeaseForBid is the error when lease is unknown for bid | ||
| ErrUnknownLeaseForBid = sdkerrors.Register(ModuleName, errCodeUnknownLeaseForOrder, "unknown lease for bid") | ||
| ErrUnknownLeaseForBid = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeUnknownLeaseForOrder, codes.NotFound, "unknown lease for bid") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check all errCodeUnknownLease constants and their usage patterns
rg -n "errCodeUnknownLease" --type=go -C 3Repository: akash-network/chain-sdk
Length of output: 1574
🏁 Script executed:
#!/bin/bash
# Check the entire file structure and search for any bid-related lease constants
cat -n go/node/market/v1/errors.go | head -30
# Also check if errCodeUnknownLeaseForBid is mentioned anywhere
rg -n "errCodeUnknownLeaseForBid" --type=go
# And check if errCodeUnknownLeaseForOrder is used elsewhere
rg -n "errCodeUnknownLeaseForOrder" --type=goRepository: akash-network/chain-sdk
Length of output: 1094
Constant name mismatch: ErrUnknownLeaseForBid uses errCodeUnknownLeaseForOrder.
The error variable refers to "bid" while the constant refers to "order". Either create a new constant errCodeUnknownLeaseForBid to match the variable and message, or review if the wrong constant was used.
🤖 Prompt for AI Agents
In @go/node/market/v1/errors.go around line 63, ErrUnknownLeaseForBid is
registered with errCodeUnknownLeaseForOrder which mismatches the variable name
and message; either add a new error code constant named
errCodeUnknownLeaseForBid and use it in sdkerrors.RegisterWithGRPCCode for
ErrUnknownLeaseForBid, or if the intent was "order" rename ErrUnknownLeaseForBid
to ErrUnknownLeaseForOrder (and update any references) so the constant
errCodeUnknownLeaseForOrder matches the error variable and message.
| ErrOrderNotFound = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeOrderNotFound, codes.NotFound, "invalid order: order not found") | ||
| // ErrGroupNotFound order not found | ||
| ErrGroupNotFound = sdkerrors.Register(ModuleName, errCodeGroupNotFound, "order not found") | ||
| ErrGroupNotFound = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeGroupNotFound, codes.NotFound, "order not found") |
There was a problem hiding this comment.
Error message and comment don't match the error name.
ErrGroupNotFound has a comment and message stating "order not found", which is inconsistent with the error name that refers to a "group".
🔎 Suggested fix
- // ErrGroupNotFound order not found
- ErrGroupNotFound = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeGroupNotFound, codes.NotFound, "order not found")
+ // ErrGroupNotFound group not found
+ ErrGroupNotFound = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeGroupNotFound, codes.NotFound, "group not found")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ErrGroupNotFound = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeGroupNotFound, codes.NotFound, "order not found") | |
| // ErrGroupNotFound group not found | |
| ErrGroupNotFound = sdkerrors.RegisterWithGRPCCode(ModuleName, errCodeGroupNotFound, codes.NotFound, "group not found") |
🤖 Prompt for AI Agents
In @go/node/market/v1/errors.go around line 77, The ErrGroupNotFound constant
incorrectly registers the message "order not found"; update the registration
call for ErrGroupNotFound (using sdkerrors.RegisterWithGRPCCode,
errCodeGroupNotFound, and codes.NotFound) to use the correct message "group not
found" and, if present, update any surrounding comment to match the "group"
semantics so the name, message, and comment are consistent.
📝 Description
Register function that was used in our error registering under the hood uses grpccodes.Unknown code, which is mapped to http.InternalError status code(500). link
So, each error ended up with that code at this line:
My fix replaces Register with RegisterWithGRPCCode that propagates the original error to the http status code.
Testing
Tested at local network with akash binary build using chain-sdk with this branch:
In browser you can see code 404, which mean that the bug is fixed.
Question to resolve
🔧 Purpose of the Change
📌 Related Issues
✅ Checklist