Skip to content

subservers: fail LiT startup when integrated sub-server boot fails #1183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ffranr wants to merge 2 commits into
base: master
Choose a base branch
from
Open

subservers: fail LiT startup when integrated sub-server boot fails #1183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f96bbe8
subservers: fail LiT startup when integrated sub-server boot fails
ffranr Dec 2, 2025
86e8ec5
docs: add release note
ffranr Dec 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions docs/release-notes/release-notes-0.16.1.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# Release Notes

- [Lightning Terminal](#lightning-terminal)
- [Bug Fixes](#bug-fixes)
- [Functional Changes/Additions](#functional-changesadditions)
- [Technical and Architectural Updates](#technical-and-architectural-updates)
- [Integrated Binary Updates](#integrated-binary-updates)
- [LND](#lnd)
- [Loop](#loop)
- [Pool](#pool)
- [Faraday](#faraday)
- [Taproot Assets](#taproot-assets)
- [Contributors](#contributors-alphabetical-order)

## Lightning Terminal

### Bug Fixes

### Functional Changes/Additions

* [PR](https://github.com/lightninglabs/lightning-terminal/pull/1183): LiT now
fails fast if any integrated sub-server cannot start. Startup errors from
integrated sub-servers are propagated directly to LiT, which aborts launch
and records the failure status.

### Technical and Architectural Updates

## RPC Updates

## Integrated Binary Updates

### LND

### Loop

### Pool

### Faraday

### Taproot Assets

# Contributors (Alphabetical Order)
5 changes: 2 additions & 3 deletions subservers/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@ import (

// SubServer defines an interface that should be implemented by any sub-server
// that the subServer manager should manage. A sub-server can be run in either
// integrated or remote mode. A sub-server is considered non-fatal to LiT
// meaning that if a sub-server fails to start, LiT can safely continue with its
// operations and other sub-servers can too.
// integrated or remote mode. Integrated sub-servers are treated as critical to
// LiT startup, meaning a failure to start any of them will abort the process.
type SubServer interface {
macaroons.MacaroonValidator

Expand Down
16 changes: 14 additions & 2 deletions subservers/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"io/ioutil"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -104,13 +105,16 @@ func (s *Manager) GetServer(name string) (SubServer, bool) {
}

// StartIntegratedServers starts all the manager's sub-servers that should be
// started in integrated mode.
// started in integrated mode. An error is returned if any integrated sub-server
// fails to start.
Comment on lines +108 to +109
Copy link
Contributor

@ViktorT-11 ViktorT-11 Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only want errors in the tapd sub-server to cause the startup process to error, and not any sub-server. The motivation for why this should specifically target tapd right now, is that we've realized that tapd is critically coupled to lnd in the scenario that the user has taproot-asset channels, in the sense that lnd can't function if tapd registers as the aux-unit, but then shuts down.

Therefore, I suggest that you add a map of "critical" sub-server names as a param to the StartIntegratedServers function. If a sub-server then errors during the startup, you'll check if that sub-server name exists in the passed map, and if so return an error.

We actually originally designed this function specifically to not error the whole startup process if any of the integrated sub-servers fails to start. The motivation behind that is that we do not want bugs in one sub-server, to disable users from using the other sub-servers until a fix for the erroring sub-server has been released. I.e. a bug that causes pool to error on startup, shouldn't disable people from performing loops until it's been fixed. Of course users can alawys disable pool in that scenario, but for UX purposes we do not want require users to understand that it's pool that's actually failing the startup process, and then additionally understand how they disable pool, as this adds friction.

func (s *Manager) StartIntegratedServers(lndClient lnrpc.LightningClient,
lndGrpc *lndclient.GrpcLndServices, withMacaroonService bool) {
lndGrpc *lndclient.GrpcLndServices, withMacaroonService bool) error {

s.mu.Lock()
defer s.mu.Unlock()

var errs []string

for _, ss := range s.servers {
if ss.Remote() {
continue
Expand All @@ -126,11 +130,19 @@ func (s *Manager) StartIntegratedServers(lndClient lnrpc.LightningClient,
)
if err != nil {
s.statusServer.SetErrored(ss.Name(), err.Error())
errs = append(errs, fmt.Sprintf("%s: %v", ss.Name(), err))
Copy link
Contributor

@ViktorT-11 ViktorT-11 Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to return an error imminently here, and not proceed to start all sub-servers before we return the error. The motivation for that is that we want lnd to shutdown asap if a "critical" sub-server errors during startup, and that could be delayed if we proceed to start the rest of the sub-servers before returning the error, and hence proceed to shutdown lnd.

continue
}

s.statusServer.SetRunning(ss.Name())
}

if len(errs) > 0 {
return fmt.Errorf("failed starting integrated sub-server(s): %s",
strings.Join(errs, "; "))
}

return nil
}

// ConnectRemoteSubServers creates connections to all the manager's sub-servers
Expand Down
5 changes: 4 additions & 1 deletion terminal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -768,9 +768,12 @@ func (g *LightningTerminal) start(ctx context.Context) error {
// Both connection types are ready now, let's start our sub-servers if
// they should be started locally as an integrated service.
createDefaultMacaroons := !g.cfg.statelessInitMode
g.subServerMgr.StartIntegratedServers(
err = g.subServerMgr.StartIntegratedServers(
g.basicClient, g.lndClient, createDefaultMacaroons,
)
if err != nil {
return fmt.Errorf("could not start integrated sub-servers: %w", err)
}
Comment on lines +774 to +776
Copy link
Contributor

@ViktorT-11 ViktorT-11 Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just returning an error here will currently actually not shutdown lnd. Instead this will cause the Run function to wait until the shutdownInterceptor.ShutdownChannel() is triggered, and keep lnd up and running until that happens. This will lead to the same errors that we've seen previously, as lnd will still proceed to try to call into tapd.

Instead we want to make sure lnd is shutdown asap here. One way of achieving that would be to call shutdownInterceptor.RequestShutdown() here. Alternatively, you could call g.basicClient.StopDaemon() here.
I suggest the second alternative, as the single Interceptor pattern is a bit of a code smell that we eventually actually want to get rid of :).


err = g.startInternalSubServers(ctx, !g.cfg.statelessInitMode)
if err != nil {
Expand Down
Loading