fix!: moved ErrInternal to the chain-sdk #2030
Conversation
WalkthroughThe changes consolidate error handling by moving the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![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.
|
|
||
| if err := ms.provider.Create(ctx, types.Provider(*msg)); err != nil { | ||
| return nil, ErrInternal.Wrapf("err: %v", err) | ||
| return nil, types.ErrInternal.Wrapf("err: %v", err) |
There was a problem hiding this comment.
Build failure: types.ErrInternal is undefined.
Static analysis confirms that types.ErrInternal is not defined, causing compilation failure. This is the same issue identified in the test file - the dependency on chain-sdk with the moved ErrInternal definition appears to be missing.
🧰 Tools
🪛 GitHub Check: build-macos
[failure] 42-42:
undefined: types.ErrInternal
🪛 golangci-lint (2.5.0)
[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)
🤖 Prompt for AI Agents
In @x/provider/handler/server.go at 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.
|
|
||
| if err := ms.provider.Update(ctx, types.Provider(*msg)); err != nil { | ||
| return nil, errorsmod.Wrapf(ErrInternal, "err: %v", err) | ||
| return nil, sdkerrors.Wrapf(types.ErrInternal, "err: %v", err) |
There was a problem hiding this comment.
Build failure and inconsistent error wrapping syntax.
Two issues:
types.ErrInternalis undefined (same build failure as lines 42 and 82).- This line uses function syntax
sdkerrors.Wrapf(types.ErrInternal, ...)while lines 42 and 82 use method syntaxtypes.ErrInternal.Wrapf(...). Prefer consistent usage throughout the file.
♻️ Standardize to method syntax (assuming types.ErrInternal becomes available)
- return nil, sdkerrors.Wrapf(types.ErrInternal, "err: %v", err)
+ return nil, types.ErrInternal.Wrapf("err: %v", err)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: build-macos
[failure] 63-63:
undefined: types.ErrInternal
🤖 Prompt for AI Agents
In @x/provider/handler/server.go at 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).
|
|
||
| // TODO: cancel leases | ||
| return nil, ErrInternal.Wrap("NOTIMPLEMENTED") | ||
| return nil, types.ErrInternal.Wrap("NOTIMPLEMENTED") |
There was a problem hiding this comment.
Build failure: types.ErrInternal is undefined.
Static analysis confirms the same compilation issue - types.ErrInternal is not available. This line correctly uses the method syntax consistent with line 42, but the error type itself must be defined before this code can build.
🧰 Tools
🪛 GitHub Check: build-macos
[failure] 82-82:
undefined: types.ErrInternal
🤖 Prompt for AI Agents
In @x/provider/handler/server.go at 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.
vertex451
changed the title
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