fix!: moved ErrInternal to the chain-sdk #2030
Conversation
WalkthroughReplaced the handler package's exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
⏰ 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). (8)
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 |
![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
Fix all issues with AI Agents 🤖
In @x/provider/handler/server_test.go:
- Around line 1-33: The file ends without a final newline causing a formatting
lint failure; run gofmt (or add a trailing newline) on the test file so the file
containing TestErrorGRPCStatusCodes and references to handler.ErrInternal is
terminated with a newline character (i.e., ensure the file ends with a blank
line) and re-run static checks.
🧹 Nitpick comments (1)
x/provider/handler/server.go (1)
48-48: Inconsistent error wrapping style across the file.The file uses two different error wrapping approaches:
- Line 48:
ErrInternal.Wrapf("err: %v", err)(method-style)- Line 69:
sdkerrors.Wrapf(ErrInternal, "err: %v", err)(function-style)- Line 88:
ErrInternal.Wrap("NOTIMPLEMENTED")(method-style)For maintainability and code consistency, all error wrapping should use the same approach.
🔎 Standardize to method-style wrapping
- return nil, sdkerrors.Wrapf(ErrInternal, "err: %v", err) + return nil, ErrInternal.Wrapf("err: %v", err)Also applies to: 69-69, 88-88
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
x/provider/handler/server.gox/provider/handler/server_test.go
🧰 Additional context used
🧬 Code graph analysis (2)
x/provider/handler/server_test.go (1)
x/provider/handler/server.go (1)
ErrInternal(18-18)
x/provider/handler/server.go (1)
x/market/alias.go (1)
ModuleName(13-13)
🪛 golangci-lint (2.5.0)
x/provider/handler/server_test.go
[error] 34-34: File is not properly formatted
(gofmt)
⏰ 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). (2)
- GitHub Check: release-dry-run
- GitHub Check: build-macos
🔇 Additional comments (2)
x/provider/handler/server.go (2)
6-7: LGTM! Proper imports for gRPC error mapping.The addition of
sdkerrorsandgoogle.golang.org/grpc/codesimports enables proper gRPC status code registration.
18-18: Excellent fix for proper HTTP/gRPC status mapping.Using
RegisterWithGRPCCodewithcodes.Internalensures this error correctly maps to HTTP 500 (Internal Server Error) instead of defaulting to an incorrect status code. This aligns with the PR objective to fix error propagation.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @x/provider/handler/server.go:
- Line 82: The build fails because server.go returns
types.ErrInternal.Wrap("NOTIMPLEMENTED") but the symbol types.ErrInternal is
undefined; fix by either (A) replacing that reference with an existing error
value or constructor used elsewhere (e.g., the project-wide internal error
constant/function) or (B) defining ErrInternal in the types package with the
same error type/shape expected here (and ensuring the Wrap method is available
or using the appropriate error-wrapping utility). Update the return in the
handler in server.go to use the correct existing error symbol or add ErrInternal
to the types package so the Wrap call compiles.
- Line 63: The error is caused by using sdkerrors.Wrapf(types.ErrInternal, ...)
and by types.ErrInternal being undefined; change this call to the same
method-syntax used elsewhere: use types.ErrInternal.Wrapf("err: %v", err). Also
ensure that the types package defines and exports ErrInternal (or import the
correct package that provides ErrInternal) so the symbol compiles; keep the
wrapping style consistent with the other usages (lines using
types.ErrInternal.Wrapf).
- Line 42: The code references types.ErrInternal which no longer exists; update
the call to use the correct error symbol and import (for example replace
types.ErrInternal.Wrapf(...) with sdkerrors.ErrInternal.Wrapf(...) and add the
import "github.com/cosmos/cosmos-sdk/types/errors" aliased as sdkerrors), or
alternatively replace the call with a local/internal error helper (e.g.,
fmt.Errorf or your package's ErrInternal) so the undefined types.ErrInternal is
removed; ensure the change is made where server.go returns nil,
types.ErrInternal.Wrapf(...) and adjust imports accordingly.
🧹 Nitpick comments (1)
x/provider/handler/server.go (1)
6-6: Import alias is fine, but usage is inconsistent.The import alias change from
errorsmodtosdkerrorsimproves clarity. However, the usage across the file is inconsistent: line 63 usessdkerrors.Wrapf(types.ErrInternal, ...)while lines 42 and 82 use method syntaxtypes.ErrInternal.Wrapf(...).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.mdx/provider/handler/handler_test.gox/provider/handler/server.go
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
🧰 Additional context used
🪛 GitHub Check: build-macos
x/provider/handler/server.go
[failure] 42-42:
undefined: types.ErrInternal
[failure] 63-63:
undefined: types.ErrInternal
[failure] 82-82:
undefined: types.ErrInternal
🪛 golangci-lint (2.5.0)
x/provider/handler/server.go
[error] 42-42: : # pkg.akt.dev/node/x/provider/handler
x/provider/handler/server.go:42:21: undefined: types.ErrInternal
x/provider/handler/server.go:63:37: undefined: types.ErrInternal
x/provider/handler/server.go:82:20: undefined: types.ErrInternal
(typecheck)
⏰ 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). (6)
- GitHub Check: lint-go
- GitHub Check: release-dry-run
- GitHub Check: build-bins
- GitHub Check: tests
- GitHub Check: sims
- GitHub Check: coverage
🔇 Additional comments (1)
x/provider/handler/handler_test.go (1)
256-257: The test code at lines 256-257 is correct.types.ErrInternalis defined in the external packagepkg.akt.dev/go/node/provider/v1beta4and is actively used throughout the codebase (e.g., inx/provider/handler/server.go). No undefined error or build failure exists.Likely an incorrect or invalid review comment.
vertex451
changed the title
| gopkg.in/yaml.v3 v3.0.1 | ||
| gotest.tools/v3 v3.5.2 | ||
| pkg.akt.dev/go v0.1.6 | ||
| pkg.akt.dev/go v0.1.9-0.20260109003838-508f70669a8c |
There was a problem hiding this comment.
I used not released commit here to pass the CI.
I am not aware of the releasing strategy, if you need anything else to finish that PR - let me know
CC: @troian
Description
Moved x/provider/handler.ErrInternal to the chain-sdk/go/node/provider/v1beta4 package.
Must be merged after akash-network/chain-sdk#181
Testing
Tested by running single node script and querying the non-existed deployment.
Author Checklist
I have...
!to the type prefix if API or client breaking changeCHANGELOG.md