frontend: alert user if spark is degraded#4164
Draft
bznein wants to merge 1 commit into
Draft
Conversation
Collaborator
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request adds Spark network status visibility to the Lightning module. The backend introduces a ✨ 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: 5
🤖 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 `@backend/lightning/lightning_test.go`:
- Around line 99-115: Add a failing-path unit test for Lightning.SparkStatus:
create a Lightning instance whose sparkStatus field is a function that returns
an error (e.g., errors.New("boom")), call lightning.SparkStatus(), and assert
that the returned status is nil and the error matches the injected error (use
require.Error/require.EqualError or require.IsType as appropriate). Ensure the
test name indicates the error path (e.g., TestSparkStatus_Error) and reference
the Lightning struct and its sparkStatus field and the SparkStatus() method when
locating where to add the test.
In `@backend/lightning/lightning.go`:
- Around line 216-218: The raw Breez SDK error is returned directly (return nil,
err), which will bypass our typed error handling in errorResponse(); instead
import and use the backend/util/errp package to wrap/translate the SDK error
before returning (e.g., replace return nil, err with return nil, errp.Wrap(err,
"breez: <brief context>") or return nil, errp.WithCode(errp.CodeX, err) to
assign an appropriate errp code), referencing the same call site in lightning.go
so downstream errorResponse() can perform typed handling.
- Around line 188-223: Add GoDoc comments for the exported type SparkStatus and
the Lightning.SparkStatus method: place a comment starting with "SparkStatus
..." immediately above the type declaring what the struct represents and
documenting its exported fields Status and LastUpdated (units/meaning for
LastUpdated), and place a comment starting with "SparkStatus ..." immediately
above the method func (lightning *Lightning) SparkStatus() describing what the
method returns, under what conditions it can return an error, and any semantics
(e.g., it queries the Spark service via breez_sdk_spark.GetSparkStatus or a test
override). Ensure comments follow GoDoc style (start with the symbol name and
are full sentences).
In `@frontends/web/src/routes/lightning/lightning.tsx`:
- Around line 67-76: The catch block in loadSparkStatus leaves sparkStatus
unchanged on errors, so update the catch to set a safe fallback Spark status
when a poll fails: inside catch, after logging the error, check mounted.current
and call setSparkStatus with a fallback object (e.g., a disabled/unknown state)
so the UI/warning banner reflects the failure; modify the loadSparkStatus
function (the async callback) to perform this fallback update in the catch path.
- Around line 88-92: Replace the hardcoded banner text inside the Status
component with a translation key: import and use the i18n translator (e.g.,
useTranslation / t) and call t('lightning.alphaWarning') (or similar) in place
of the literal string in the Status instance that has
dismissibleKey="lightning-alpha-warning"; then add the corresponding
"lightning.alphaWarning" entry to src/locales/{lang}/app.json for each supported
language with the original message. Ensure the translation key name matches
across the component and locale files.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 82e4eb62-32e1-4242-b95d-696145c0761e
📒 Files selected for processing (6)
backend/lightning/handlers.gobackend/lightning/lightning.gobackend/lightning/lightning_test.gofrontends/web/src/api/lightning.tsfrontends/web/src/locales/en/app.jsonfrontends/web/src/routes/lightning/lightning.tsx
Comment on lines
+99
to
+115
| func TestSparkStatus(t *testing.T) { | ||
| lightning := &Lightning{ | ||
| sparkStatus: func() (breez_sdk_spark.SparkStatus, error) { | ||
| return breez_sdk_spark.SparkStatus{ | ||
| Status: breez_sdk_spark.ServiceStatusPartial, | ||
| LastUpdated: 1713790323, | ||
| }, nil | ||
| }, | ||
| } | ||
|
|
||
| status, err := lightning.SparkStatus() | ||
| require.NoError(t, err) | ||
| require.Equal(t, &SparkStatus{ | ||
| Status: "partial", | ||
| LastUpdated: 1713790323, | ||
| }, status) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add an explicit failure-path test for Lightning.SparkStatus().
Current coverage validates only the success path. Please add a case where the injected sparkStatus function returns an error and assert that Lightning.SparkStatus() returns (nil, err).
🤖 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 `@backend/lightning/lightning_test.go` around lines 99 - 115, Add a
failing-path unit test for Lightning.SparkStatus: create a Lightning instance
whose sparkStatus field is a function that returns an error (e.g.,
errors.New("boom")), call lightning.SparkStatus(), and assert that the returned
status is nil and the error matches the injected error (use
require.Error/require.EqualError or require.IsType as appropriate). Ensure the
test name indicates the error path (e.g., TestSparkStatus_Error) and reference
the Lightning struct and its sparkStatus field and the SparkStatus() method when
locating where to add the test.
Comment on lines
+188
to
+223
| type SparkStatus struct { | ||
| Status string `json:"status"` | ||
| LastUpdated uint64 `json:"lastUpdated"` | ||
| } | ||
|
|
||
| func serviceStatus(status breez_sdk_spark.ServiceStatus) string { | ||
| switch status { | ||
| case breez_sdk_spark.ServiceStatusOperational: | ||
| return "operational" | ||
| case breez_sdk_spark.ServiceStatusDegraded: | ||
| return "degraded" | ||
| case breez_sdk_spark.ServiceStatusPartial: | ||
| return "partial" | ||
| case breez_sdk_spark.ServiceStatusMajor: | ||
| return "major" | ||
| case breez_sdk_spark.ServiceStatusUnknown: | ||
| return "unknown" | ||
| default: | ||
| return "unknown" | ||
| } | ||
| } | ||
|
|
||
| func (lightning *Lightning) SparkStatus() (*SparkStatus, error) { | ||
| getSparkStatus := lightning.sparkStatus | ||
| if getSparkStatus == nil { | ||
| getSparkStatus = breez_sdk_spark.GetSparkStatus | ||
| } | ||
| status, err := getSparkStatus() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return &SparkStatus{ | ||
| Status: serviceStatus(status.Status), | ||
| LastUpdated: status.LastUpdated, | ||
| }, nil | ||
| } |
There was a problem hiding this comment.
Add GoDoc comments for the new exported API.
SparkStatus (type) and SparkStatus() (method) are exported but undocumented, which is a common golangci-lint failure point for backend packages.
💡 Proposed fix
+// SparkStatus represents the current Spark network status.
type SparkStatus struct {
Status string `json:"status"`
LastUpdated uint64 `json:"lastUpdated"`
}
@@
+// SparkStatus retrieves the current Spark network status.
func (lightning *Lightning) SparkStatus() (*SparkStatus, error) {📝 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.
Suggested change
| type SparkStatus struct { | |
| Status string `json:"status"` | |
| LastUpdated uint64 `json:"lastUpdated"` | |
| } | |
| func serviceStatus(status breez_sdk_spark.ServiceStatus) string { | |
| switch status { | |
| case breez_sdk_spark.ServiceStatusOperational: | |
| return "operational" | |
| case breez_sdk_spark.ServiceStatusDegraded: | |
| return "degraded" | |
| case breez_sdk_spark.ServiceStatusPartial: | |
| return "partial" | |
| case breez_sdk_spark.ServiceStatusMajor: | |
| return "major" | |
| case breez_sdk_spark.ServiceStatusUnknown: | |
| return "unknown" | |
| default: | |
| return "unknown" | |
| } | |
| } | |
| func (lightning *Lightning) SparkStatus() (*SparkStatus, error) { | |
| getSparkStatus := lightning.sparkStatus | |
| if getSparkStatus == nil { | |
| getSparkStatus = breez_sdk_spark.GetSparkStatus | |
| } | |
| status, err := getSparkStatus() | |
| if err != nil { | |
| return nil, err | |
| } | |
| return &SparkStatus{ | |
| Status: serviceStatus(status.Status), | |
| LastUpdated: status.LastUpdated, | |
| }, nil | |
| } | |
| // SparkStatus represents the current Spark network status. | |
| type SparkStatus struct { | |
| Status string `json:"status"` | |
| LastUpdated uint64 `json:"lastUpdated"` | |
| } | |
| func serviceStatus(status breez_sdk_spark.ServiceStatus) string { | |
| switch status { | |
| case breez_sdk_spark.ServiceStatusOperational: | |
| return "operational" | |
| case breez_sdk_spark.ServiceStatusDegraded: | |
| return "degraded" | |
| case breez_sdk_spark.ServiceStatusPartial: | |
| return "partial" | |
| case breez_sdk_spark.ServiceStatusMajor: | |
| return "major" | |
| case breez_sdk_spark.ServiceStatusUnknown: | |
| return "unknown" | |
| default: | |
| return "unknown" | |
| } | |
| } | |
| // SparkStatus retrieves the current Spark network status. | |
| func (lightning *Lightning) SparkStatus() (*SparkStatus, error) { | |
| getSparkStatus := lightning.sparkStatus | |
| if getSparkStatus == nil { | |
| getSparkStatus = breez_sdk_spark.GetSparkStatus | |
| } | |
| status, err := getSparkStatus() | |
| if err != nil { | |
| return nil, err | |
| } | |
| return &SparkStatus{ | |
| Status: serviceStatus(status.Status), | |
| LastUpdated: status.LastUpdated, | |
| }, nil | |
| } |
🤖 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 `@backend/lightning/lightning.go` around lines 188 - 223, Add GoDoc comments
for the exported type SparkStatus and the Lightning.SparkStatus method: place a
comment starting with "SparkStatus ..." immediately above the type declaring
what the struct represents and documenting its exported fields Status and
LastUpdated (units/meaning for LastUpdated), and place a comment starting with
"SparkStatus ..." immediately above the method func (lightning *Lightning)
SparkStatus() describing what the method returns, under what conditions it can
return an error, and any semantics (e.g., it queries the Spark service via
breez_sdk_spark.GetSparkStatus or a test override). Ensure comments follow GoDoc
style (start with the symbol name and are full sentences).
Comment on lines
+216
to
+218
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
Do not return raw SDK errors across the backend boundary.
Line 217 returns the Breez SDK error as-is; downstream errorResponse() will expose err.Error() and skip typed error-code handling. Please wrap/translate this into an errp coded error before returning.
As per coding guidelines: backend/**/*.go: Use errp package in backend/util/errp/ for error wrapping and typed error codes.
🤖 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 `@backend/lightning/lightning.go` around lines 216 - 218, The raw Breez SDK
error is returned directly (return nil, err), which will bypass our typed error
handling in errorResponse(); instead import and use the backend/util/errp
package to wrap/translate the SDK error before returning (e.g., replace return
nil, err with return nil, errp.Wrap(err, "breez: <brief context>") or return
nil, errp.WithCode(errp.CodeX, err) to assign an appropriate errp code),
referencing the same call site in lightning.go so downstream errorResponse() can
perform typed handling.
Comment on lines
+88
to
+92
| <Status | ||
| dismissibleKey="lightning-alpha-warning" | ||
| type="warning"> | ||
| This is an alpha release intended for preview and testing. Only use lightning with a small amount of funds! | ||
| </Status> |
There was a problem hiding this comment.
Move the alpha warning text into i18n keys.
This banner message is hardcoded in the component instead of using a translation key.
💡 Proposed fix
<Status
dismissibleKey="lightning-alpha-warning"
type="warning">
- This is an alpha release intended for preview and testing. Only use lightning with a small amount of funds!
+ {t('lightning.alphaWarning')}
</Status> "lightning": {
+ "alphaWarning": "This is an alpha release intended for preview and testing. Only use lightning with a small amount of funds!",
"accountLabel": "Lightning",
"activate": {🤖 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 `@frontends/web/src/routes/lightning/lightning.tsx` around lines 88 - 92,
Replace the hardcoded banner text inside the Status component with a translation
key: import and use the i18n translator (e.g., useTranslation / t) and call
t('lightning.alphaWarning') (or similar) in place of the literal string in the
Status instance that has dismissibleKey="lightning-alpha-warning"; then add the
corresponding "lightning.alphaWarning" entry to src/locales/{lang}/app.json for
each supported language with the original message. Ensure the translation key
name matches across the component and locale files.
Collaborator
Author
There was a problem hiding this comment.
This has not been introduced in this PR. Will fix it in another one
There was a problem hiding this comment.
@bznein, understood! Since this pre-existing issue will be addressed in a follow-up PR, would you like me to open a GitHub issue to track the fix (moving the alpha warning text to an i18n key)?
🧠 Learnings used
Learnt from: thisconnect
Repo: BitBoxSwiss/bitbox-wallet-app PR: 4038
File: frontends/web/src/routes/settings/components/about/support-link-setting.tsx:7-7
Timestamp: 2026-04-22T18:36:13.978Z
Learning: In files under `frontends/web/src/routes/`, do not flag relative imports within the `src/routes/` directory tree (e.g., `./...` and `../...`). Use the `@/` alias mainly when the import leaves `src/routes/` and targets other `src/` areas (e.g., `@/components`, `@/api`, `@/utils`).
Learnt from: thisconnect
Repo: BitBoxSwiss/bitbox-wallet-app PR: 4102
File: frontends/web/src/components/infobutton/infobutton.tsx:6-6
Timestamp: 2026-05-05T13:22:38.824Z
Learning: In this repository’s web frontend, accessibility (screen reader support and meaningful ARIA semantics such as aria-labels/roles) is intentionally deferred until an active accessibility effort starts. During code reviews, do not raise or require missing ARIA attributes, roles, or screen-reader support as issues in TSX changes; wait to enforce these concerns once accessibility work is underway.
Learnt from: thisconnect
Repo: BitBoxSwiss/bitbox-wallet-app PR: 3916
File: frontends/web/src/components/banners/offline-error.tsx:16-17
Timestamp: 2026-05-19T14:46:15.693Z
Learning: In BitBoxSwiss/bitbox-wallet-app, treat `proxy` as optional on `TBackendConfig` (defined in `frontends/web/src/api/config.ts`). When accessing `useProxy`, use the correct optional-chaining pattern: `config?.backend.proxy?.useProxy` (because `config` may be `undefined`, `backend` is required once `config` exists, and `proxy` may be missing). Do not flag code during review for not treating `proxy` as a required field of `TBackendConfig`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Before asking for reviews, here is a check list of the most common things you might need to consider: