feat: add offline mode to cli propagated through context #118
Conversation
WalkthroughAdds a sentinel error for missing context values, wraps context-access errors with it, guards client discovery behind offline checks, adds a nil-check for certQuerier in provider TLS verification, and updates CLI broadcast paths and tests to propagate the new wrapped errors. Changes
Sequence DiagramsequenceDiagram
participant User
participant Broadcast as broadcast.RunE
participant Context as context.ClientFromContext
participant TxInit as tx.InitClientContext
participant Discovery as discovery.DiscoverClient
participant Provider as Provider/Network
User->>Broadcast: run broadcast command
Broadcast->>Context: ClientFromContext(ctx)
alt client present
Context-->>Broadcast: (client, nil)
else missing
Context-->>Broadcast: (nil, ErrContextValueNotSet)
Broadcast-->>User: return error
end
Broadcast->>TxInit: Init client context using client
alt not offline
TxInit->>Discovery: DiscoverClient(ctx, cctx, opts...)
Discovery->>Provider: network discovery
alt success
Discovery-->>TxInit: (client, nil)
else fail
Discovery-->>TxInit: (nil, error)
TxInit-->>Broadcast: return error
Broadcast-->>User: return error
end
else offline
TxInit-->>Broadcast: use zero-value client (no discovery)
end
Broadcast->>Broadcast: process and broadcast transaction
Broadcast-->>User: success or tx error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ 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)
🔇 Additional comments (1)
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: 4
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
go/cli/cctx.go(1 hunks)go/cli/context.go(5 hunks)go/cli/flags/flags.go(2 hunks)go/cli/query.go(1 hunks)go/cli/tx.go(3 hunks)go/node/client/client.go(1 hunks)go/provider/client/client.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
go/cli/cctx.go (1)
go/cli/context.go (1)
ContextTypeProviderURL(29-29)
go/cli/tx.go (3)
go/cli/flags/flags.go (1)
FlagProviderURL(89-89)go/cli/context.go (3)
ContextTypeProviderURL(29-29)ContextTypeClient(23-23)ContextTypeQueryClient(24-24)go/node/client/client.go (2)
DiscoverClient(78-102)DiscoverQueryClient(135-159)
go/cli/query.go (2)
go/cli/flags/flags.go (1)
FlagProviderURL(89-89)go/cli/context.go (1)
ContextTypeProviderURL(29-29)
go/provider/client/client.go (1)
go/util/tls/verify.go (2)
CertificateInvalidError(29-32)Expired(23-23)
⏰ 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: go
- GitHub Check: test
- GitHub Check: coverage
🔇 Additional comments (2)
go/node/client/client.go (1)
137-139: LGTM! Critical bug fix that prevents nil pointer dereference.This early return is essential and makes
DiscoverQueryClientconsistent withDiscoverClient(lines 79-82) andDiscoverLightClient(lines 105-108). Without this check, ifqueryClientInforeturns an error,resultwould benil, causing a panic when accessingresult.ClientInfo.ApiVersionon line 143.go/cli/tx.go (1)
10-11: LGTM! Import refactoring aligns with the new discovery pattern.The imports correctly separate the core client types (
aclient) from the discovery functions (discovery), which aligns with the relevant code snippets showingDiscoverClientandDiscoverQueryClientin the discovery package.
cloud-j-luna
changed the title
cloud-j-luna
force-pushed
the
luna/offline-mode-context
branch
from
f1139de to
b88cf74
Compare
github-actions
bot
added
Typ: Docs
C:ts
C:go/sdl
and removed
Typ: Docs
C:ts
C:go/sdl
labels
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go/cli/context.go (1)
99-111: Consider enhancing type mismatch panic messages for consistency.The updates to lines 102 and 116 improve error clarity by using
fmt.ErrorfwithErrContextValueNotSet. For additional consistency with the pattern used inClientFromContext(line 42), consider updating the type mismatch panics on lines 107 and 121 to include the actual type information:// Line 107 panic(fmt.Errorf("invalid context value, expected \"address.Codec\", actual \"%s\"", reflect.TypeOf(val))) // Line 121 panic(fmt.Errorf("invalid context value, expected \"address.Codec\", actual \"%s\"", reflect.TypeOf(val)))This would make debugging easier by showing what type was actually present when the assertion fails.
Also applies to: 113-125
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
go/cli/context.go(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: cloud-j-luna
PR: akash-network/chain-sdk#118
File: go/cli/tx.go:23-27
Timestamp: 2025-10-16T10:30:51.243Z
Learning: In the chain-sdk repository (go/cli/tx.go and go/cli/query.go), the ContextTypeProviderURL context value is intentionally set for consumption by external codebases, specifically the provider codebase, even though it's not consumed within chain-sdk itself.
📚 Learning: 2025-10-16T10:30:51.243Z
Learnt from: cloud-j-luna
PR: akash-network/chain-sdk#118
File: go/cli/tx.go:23-27
Timestamp: 2025-10-16T10:30:51.243Z
Learning: In the chain-sdk repository (go/cli/tx.go and go/cli/query.go), the ContextTypeProviderURL context value is intentionally set for consumption by external codebases, specifically the provider codebase, even though it's not consumed within chain-sdk itself.
Applied to files:
go/cli/context.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: test
- GitHub Check: coverage
- GitHub Check: go
🔇 Additional comments (3)
go/cli/context.go (3)
29-29: LGTM! Provider URL context type added as intended.The new
ContextTypeProviderURLconstant follows the existing pattern and aligns with the PR's objective of propagating provider URL through context for external consumption.Based on learnings.
32-32: Excellent error handling improvement.The introduction of
ErrContextValueNotSetas a sentinel error is a best practice that enables callers to detect missing context values usingerrors.Is(). The consistent wrapping pattern across all accessor functions (ClientFromContext,QueryClientFromContext,LightClientFromContext,MustAddressCodecFromContext,MustValidatorCodecFromContext) provides both context-specific information and error type detection.Also applies to: 37-37, 51-51, 76-76, 102-102, 116-116
48-60: Well-implemented QueryClient accessor.The
QueryClientFromContextfunction follows the established pattern with proper error handling:
- Returns wrapped
ErrContextValueNotSetfor missing values- Performs type assertion with descriptive error messages
- Correctly references
"aclient.QueryClient"in the error message
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
go/cli/broadcast.go (1)
45-45: Consider using non-Must variant for defensive programming.While
MustClientFromContextis safe here (the offline check at line 36 ensures we're not in offline mode, and the client should be available in context), usingClientFromContextwith explicit error handling would be more defensive and consistent with the codebase pattern.Apply this diff:
-cl := MustClientFromContext(ctx) +cl, err := ClientFromContext(ctx) +if err != nil { + return err +}Based on learnings: operations should prefer checking for
ErrContextValueNotSetusing non-Must variants, though the broadcast operation's explicit offline check makes the panic unlikely.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
go/cli/broadcast.go(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: cloud-j-luna
PR: akash-network/chain-sdk#118
File: go/cli/tx.go:48-63
Timestamp: 2025-10-16T10:32:01.585Z
Learning: In the Akash chain-sdk CLI (Go), when `cctx.Offline` is true, nil clients are intentionally stored in the context using `ContextTypeClient` and `ContextTypeQueryClient` keys. Operations that support offline mode should check for `ErrContextValueNotSet` when retrieving clients from context using `ClientFromContext()` or `QueryClientFromContext()`, rather than using the `Must*` variants which panic on nil values.
Learnt from: cloud-j-luna
PR: akash-network/chain-sdk#118
File: go/cli/tx.go:23-27
Timestamp: 2025-10-16T10:30:51.243Z
Learning: In the chain-sdk repository (go/cli/tx.go and go/cli/query.go), the ContextTypeProviderURL context value is intentionally set for consumption by external codebases, specifically the provider codebase, even though it's not consumed within chain-sdk itself.
🧬 Code graph analysis (1)
go/cli/broadcast.go (2)
go/cli/cctx.go (1)
GetClientTxContext(81-84)go/cli/context.go (1)
MustClientFromContext(62-69)
⏰ 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: proto
- GitHub Check: go
- GitHub Check: test
- GitHub Check: coverage
🔇 Additional comments (1)
go/cli/broadcast.go (1)
31-34: LGTM: Proper error handling for context retrieval.The explicit error handling for
GetClientTxContextis a clear improvement over any previous direct retrieval pattern.
cloud-j-luna
changed the title
cloud-j-luna
force-pushed
the
luna/offline-mode-context
branch
2 times, most recently
from
07d164a to
d730edc
Compare
- Add offline flag support to CLI commands - Propagate offline mode through context to prevent network calls - Fix broadcast command to check offline mode before accessing client - Add context value helpers for client and query client management - Update client initialization to skip when in offline mode
cloud-j-luna
force-pushed
the
luna/offline-mode-context
branch
from
297a2cf to
1c86640
Compare
📝 Description
[Explain what this PR does in 2-3 sentences. Include context about the feature or problem being solved]
🔧 Purpose of the Change
📌 Related Issues
✅ Checklist
📎 Notes for Reviewers
[Include any additional context, architectural decisions, or specific areas to focus on]