fix(plugins): surface host service failures when loading plugins (#5756)

* fix(plugins): surface host service failures when loading plugins

When a host service failed to initialize during plugin load (e.g. the
taskqueue database could not be created because the data folder is not
writable), the error was logged and swallowed, and its host functions were
silently omitted. Instantiation then failed with a misleading error such as
'"task_createqueue" is not exported in module "extism:host/user"', which
reads as a plugin/host API mismatch and gets wrongly blamed on plugin
authors (see kgarner7/navidrome-listenbrainz-daily-playlist#26).

Host service factories now return an error, and loadPluginWithConfig fails
fast with the actual cause (e.g. 'creating Task service: creating plugin
data directory: ...'), which is also stored in the plugin's last_error.
Closers accumulated before a load failure are now closed (via a deferred
guard covering all failure paths), so partially-created services no longer
leak goroutines or database handles.

Also fix a panic in 'navidrome plugin enable': CLI commands use the plugin
Manager without calling Start, so manager.ctx was nil and
newTaskQueueService panicked in context.WithCancel. Long-lived host
services (taskqueue, kvstore, websocket) now receive their lifecycle
context explicitly in the constructor, sourced from serviceContext.baseCtx(),
which falls back to context.Background() for the unstarted-manager case.

* docs(plugins): correct websocket readLoop lifecycle comment

The comment claimed the read loop's context is always cancelled during
application shutdown, which is not true when the manager was never started
(one-shot CLI runs, where baseCtx falls back to context.Background()).
Clarify that connection closure via Close() on plugin unload is what ends
the read loop, with context cancellation as a server-shutdown backstop.

Addresses review feedback on #5756.

* test(plugins): exclude plugin-loading specs from Windows builds

The new loadPluginWithConfig specs reference test suite helpers
(testdataDir, noopMetricsRecorder) defined in plugins_suite_test.go, which
is excluded on Windows, breaking test compilation there. Move the specs to
their own file with the same build constraint, keeping the pure-function
specs in manager_loader_test.go running on Windows as before.
This commit is contained in:
Deluan Quintão 2026-07-10 19:15:49 -04:00 committed by GitHub
parent 116a440718
commit 205c85da55
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 158 additions and 54 deletions

View File

@ -82,7 +82,8 @@ type taskQueueServiceImpl struct {
}
// newTaskQueueService creates a new taskQueueServiceImpl with its own SQLite database.
func newTaskQueueService(pluginName string, manager *Manager, maxConcurrency int32) (*taskQueueServiceImpl, error) {
// The given ctx bounds the service's background work (queue workers, cleanup loop).
func newTaskQueueService(ctx context.Context, pluginName string, manager *Manager, maxConcurrency int32) (*taskQueueServiceImpl, error) {
dataDir := filepath.Join(conf.Server.DataFolder.String(), "plugins", pluginName)
if err := os.MkdirAll(dataDir, 0700); err != nil {
return nil, fmt.Errorf("creating plugin data directory: %w", err)
@ -102,7 +103,7 @@ func newTaskQueueService(pluginName string, manager *Manager, maxConcurrency int
return nil, fmt.Errorf("creating taskqueue schema: %w", err)
}
ctx, cancel := context.WithCancel(manager.ctx) //nolint:gosec // cancel is stored in struct and called in Close()
ctx, cancel := context.WithCancel(ctx) //nolint:gosec // cancel is stored in struct and called in Close()
s := &taskQueueServiceImpl{
pluginName: pluginName,

View File

@ -42,15 +42,11 @@ var _ = Describe("TaskQueueService", func() {
DeferCleanup(configtest.SetupConfig())
conf.Server.DataFolder = conf.NewDir(tmpDir)
// Create a mock manager with context
managerCtx, cancel := context.WithCancel(ctx)
manager = &Manager{
plugins: make(map[string]*plugin),
ctx: managerCtx,
}
DeferCleanup(cancel)
service, err = newTaskQueueService("test_plugin", manager, 5)
service, err = newTaskQueueService(ctx, "test_plugin", manager, 5)
Expect(err).ToNot(HaveOccurred())
})
@ -730,14 +726,11 @@ var _ = Describe("TaskQueueService", func() {
service.Close()
// Create a new service pointing to the same DB
managerCtx2, cancel2 := context.WithCancel(ctx)
DeferCleanup(cancel2)
manager2 := &Manager{
plugins: make(map[string]*plugin),
ctx: managerCtx2,
}
service, err = newTaskQueueService("test_plugin", manager2, 5)
service, err = newTaskQueueService(ctx, "test_plugin", manager2, 5)
Expect(err).ToNot(HaveOccurred())
// Override callback to succeed
@ -775,14 +768,11 @@ var _ = Describe("TaskQueueService", func() {
Describe("Plugin isolation", func() {
It("uses separate databases for different plugins", func() {
managerCtx2, cancel2 := context.WithCancel(ctx)
DeferCleanup(cancel2)
manager2 := &Manager{
plugins: make(map[string]*plugin),
ctx: managerCtx2,
}
service2, err := newTaskQueueService("other_plugin", manager2, 5)
service2, err := newTaskQueueService(ctx, "other_plugin", manager2, 5)
Expect(err).ToNot(HaveOccurred())
defer service2.Close()

View File

@ -54,6 +54,7 @@ type wsConnection struct {
// webSocketServiceImpl implements host.WebSocketService.
// It provides plugins with WebSocket communication capabilities.
type webSocketServiceImpl struct {
baseCtx context.Context // bounds the read loops, which outlive the Connect() call
pluginName string
manager *Manager
requiredHosts []string
@ -63,8 +64,9 @@ type webSocketServiceImpl struct {
}
// newWebSocketService creates a new WebSocketService for a plugin.
func newWebSocketService(pluginName string, manager *Manager, permission *WebSocketPermission) *webSocketServiceImpl {
func newWebSocketService(ctx context.Context, pluginName string, manager *Manager, permission *WebSocketPermission) *webSocketServiceImpl {
return &webSocketServiceImpl{
baseCtx: ctx,
pluginName: pluginName,
manager: manager,
requiredHosts: permission.RequiredHosts,
@ -129,11 +131,12 @@ func (s *webSocketServiceImpl) Connect(ctx context.Context, urlStr string, heade
s.connections[connectionID] = wsConn
s.mu.Unlock()
// Start read goroutine with manager's context.
// We use manager.ctx instead of the caller's ctx because the readLoop must
// outlive the Connect() call. The manager's context is cancelled during
// application shutdown, ensuring graceful cleanup.
go s.readLoop(s.manager.ctx, connectionID, wsConn)
// Start read goroutine with the service's base context instead of the
// caller's ctx, because the readLoop must outlive the Connect() call.
// Connections are closed by Close() when the plugin is unloaded, which ends
// the readLoop; the base context is a backstop that also ends it on server
// shutdown (it is never cancelled in one-shot CLI runs).
go s.readLoop(s.baseCtx, connectionID, wsConn)
log.Debug(ctx, "WebSocket connected", "plugin", s.pluginName, "connectionID", connectionID, "url", urlStr)
return connectionID, nil

View File

@ -30,11 +30,23 @@ type serviceContext struct {
allLibraries bool // If true, plugin can access all libraries
}
// baseCtx returns the manager's lifecycle context, for host services that
// outlive the plugin call that created them. It falls back to
// context.Background() when the manager was never started, which is the case
// for CLI commands (e.g. `navidrome plugin enable`) that load plugins without
// calling Start.
func (c *serviceContext) baseCtx() context.Context {
if c.manager.ctx == nil {
return context.Background()
}
return c.manager.ctx
}
// hostServiceEntry defines a host service for table-driven registration.
type hostServiceEntry struct {
name string
hasPermission func(*Permissions) bool
create func(*serviceContext) ([]extism.HostFunction, io.Closer)
create func(*serviceContext) ([]extism.HostFunction, io.Closer, error)
}
// hostServices defines all available host services.
@ -43,119 +55,117 @@ var hostServices = []hostServiceEntry{
{
name: "Config",
hasPermission: func(p *Permissions) bool { return true }, // Always available, no permission required
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
service := newConfigService(ctx.pluginName, ctx.config)
return host.RegisterConfigHostFunctions(service), nil
return host.RegisterConfigHostFunctions(service), nil, nil
},
},
{
name: "SubsonicAPI",
hasPermission: func(p *Permissions) bool { return p != nil && p.Subsonicapi != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
service := newSubsonicAPIService(ctx.pluginName, ctx.manager.subsonicRouter, ctx.manager.ds, newUserAccess(ctx.allowedUsers, ctx.allUsers))
return host.RegisterSubsonicAPIHostFunctions(service), nil
return host.RegisterSubsonicAPIHostFunctions(service), nil, nil
},
},
{
name: "Scheduler",
hasPermission: func(p *Permissions) bool { return p != nil && p.Scheduler != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
service := newSchedulerService(ctx.pluginName, ctx.manager, scheduler.GetInstance())
return host.RegisterSchedulerHostFunctions(service), service
return host.RegisterSchedulerHostFunctions(service), service, nil
},
},
{
name: "WebSocket",
hasPermission: func(p *Permissions) bool { return p != nil && p.Websocket != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
perm := ctx.permissions.Websocket
service := newWebSocketService(ctx.pluginName, ctx.manager, perm)
return host.RegisterWebSocketHostFunctions(service), service
service := newWebSocketService(ctx.baseCtx(), ctx.pluginName, ctx.manager, perm)
return host.RegisterWebSocketHostFunctions(service), service, nil
},
},
{
name: "Artwork",
hasPermission: func(p *Permissions) bool { return p != nil && p.Artwork != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
service := newArtworkService()
return host.RegisterArtworkHostFunctions(service), nil
return host.RegisterArtworkHostFunctions(service), nil, nil
},
},
{
name: "Cache",
hasPermission: func(p *Permissions) bool { return p != nil && p.Cache != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
service := newCacheService(ctx.pluginName)
return host.RegisterCacheHostFunctions(service), service
return host.RegisterCacheHostFunctions(service), service, nil
},
},
{
name: "Library",
hasPermission: func(p *Permissions) bool { return p != nil && p.Library != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
perm := ctx.permissions.Library
service := newLibraryService(ctx.manager.ds, perm, ctx.allowedLibraries, ctx.allLibraries)
return host.RegisterLibraryHostFunctions(service), nil
return host.RegisterLibraryHostFunctions(service), nil, nil
},
},
{
name: "KVStore",
hasPermission: func(p *Permissions) bool { return p != nil && p.Kvstore != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
perm := ctx.permissions.Kvstore
service, err := newKVStoreService(ctx.manager.ctx, ctx.pluginName, perm)
service, err := newKVStoreService(ctx.baseCtx(), ctx.pluginName, perm)
if err != nil {
log.Error("Failed to create KVStore service", "plugin", ctx.pluginName, err)
return nil, nil
return nil, nil, err
}
return host.RegisterKVStoreHostFunctions(service), service
return host.RegisterKVStoreHostFunctions(service), service, nil
},
},
{
name: "Users",
hasPermission: func(p *Permissions) bool { return p != nil && p.Users != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
service := newUsersService(ctx.manager.ds, ctx.allowedUsers, ctx.allUsers)
return host.RegisterUsersHostFunctions(service), nil
return host.RegisterUsersHostFunctions(service), nil, nil
},
},
{
name: "Matcher",
hasPermission: func(p *Permissions) bool { return p != nil && p.Matcher != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
hasFilesystemPerm := ctx.permissions.Library != nil && ctx.permissions.Library.Filesystem
service := newMatcherService(
ctx.manager.ds, hasFilesystemPerm,
newUserAccess(ctx.allowedUsers, ctx.allUsers),
newLibraryAccess(ctx.allowedLibraries, ctx.allLibraries),
)
return host.RegisterMatcherHostFunctions(service), nil
return host.RegisterMatcherHostFunctions(service), nil, nil
},
},
{
name: "HTTP",
hasPermission: func(p *Permissions) bool { return p != nil && p.Http != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
perm := ctx.permissions.Http
service := newHTTPService(ctx.pluginName, perm)
return host.RegisterHTTPHostFunctions(service), nil
return host.RegisterHTTPHostFunctions(service), nil, nil
},
},
{
name: "Task",
hasPermission: func(p *Permissions) bool { return p != nil && p.Taskqueue != nil },
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer) {
create: func(ctx *serviceContext) ([]extism.HostFunction, io.Closer, error) {
perm := ctx.permissions.Taskqueue
maxConcurrency := int32(1)
if perm.MaxConcurrency > 0 {
maxConcurrency = int32(perm.MaxConcurrency)
}
service, err := newTaskQueueService(ctx.pluginName, ctx.manager, maxConcurrency)
service, err := newTaskQueueService(ctx.baseCtx(), ctx.pluginName, ctx.manager, maxConcurrency)
if err != nil {
log.Error("Failed to create Task service", "plugin", ctx.pluginName, err)
return nil, nil
return nil, nil, err
}
return host.RegisterTaskHostFunctions(service), service
return host.RegisterTaskHostFunctions(service), service, nil
},
},
}
@ -256,6 +266,7 @@ func (m *Manager) loadEnabledPlugins(ctx context.Context) error {
// loadPluginWithConfig loads a plugin with configuration from DB.
// The p.Path should point to an .ndp package file.
func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
// NewContext falls back to context.Background() when m.ctx is nil (unstarted manager)
ctx := log.NewContext(m.ctx, "plugin", p.ID)
if m.stopped.Load() {
@ -328,6 +339,15 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
// Build host functions based on permissions from manifest
var hostFunctions []extism.HostFunction
var closers []io.Closer
loaded := false
// On success the closers are owned by the registered plugin; on any
// failure past this point, close them so partially-created services
// don't leak goroutines or file handles.
defer func() {
if !loaded {
closeAll(closers)
}
}()
svcCtx := &serviceContext{
pluginName: p.ID,
@ -341,7 +361,10 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
}
for _, entry := range hostServices {
if entry.hasPermission(pkg.Manifest.Permissions) {
funcs, closer := entry.create(svcCtx)
funcs, closer, err := entry.create(svcCtx)
if err != nil {
return fmt.Errorf("creating %s service: %w", entry.name, err)
}
hostFunctions = append(hostFunctions, funcs...)
if closer != nil {
closers = append(closers, closer)
@ -400,6 +423,7 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
libraries: newLibraryAccess(allowedLibraries, p.AllLibraries),
}
m.mu.Unlock()
loaded = true
// Call plugin init function
callPluginInit(ctx, m.plugins[p.ID])
@ -407,6 +431,14 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error {
return nil
}
// closeAll closes host service closers accumulated before a load failure,
// so partially-created services don't leak goroutines or file handles.
func closeAll(closers []io.Closer) {
for _, c := range closers {
_ = c.Close()
}
}
// parsePluginConfig parses a JSON config string into a map of string values.
// For Extism, all config values must be strings, so non-string values are serialized as JSON.
func parsePluginConfig(configJSON string) (map[string]string, error) {

View File

@ -0,0 +1,78 @@
//go:build !windows
package plugins
import (
"crypto/sha256"
"encoding/hex"
"net/http"
"os"
"path/filepath"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("loadPluginWithConfig", func() {
var manager *Manager
var dataDir string
BeforeEach(func() {
pluginsDir := GinkgoT().TempDir()
dataDir = GinkgoT().TempDir()
src := filepath.Join(testdataDir, "test-taskqueue"+PackageExtension)
data, err := os.ReadFile(src)
Expect(err).ToNot(HaveOccurred())
dest := filepath.Join(pluginsDir, "test-taskqueue"+PackageExtension)
Expect(os.WriteFile(dest, data, 0600)).To(Succeed())
hash := sha256.Sum256(data)
DeferCleanup(configtest.SetupConfig())
conf.Server.Plugins.Enabled = true
conf.Server.Plugins.Folder = conf.NewDir(pluginsDir)
conf.Server.Plugins.AutoReload = false
conf.Server.DataFolder = conf.NewDir(dataDir)
repo := tests.CreateMockPluginRepo()
repo.Permitted = true
repo.SetData(model.Plugins{{
ID: "test-taskqueue",
Path: dest,
SHA256: hex.EncodeToString(hash[:]),
Enabled: false,
}})
manager = &Manager{
plugins: make(map[string]*plugin),
ds: &tests.MockDataStore{MockedPlugin: repo},
metrics: noopMetricsRecorder{},
subsonicRouter: http.NotFoundHandler(),
}
})
Describe("host service creation failures", func() {
It("reports the Task service creation error instead of a missing host function", func() {
Expect(manager.Start(GinkgoT().Context())).To(Succeed())
DeferCleanup(func() { _ = manager.Stop() })
// Block the taskqueue data dir by creating a file where the directory should be
Expect(os.WriteFile(filepath.Join(dataDir, "plugins"), nil, 0600)).To(Succeed())
err := manager.EnablePlugin(GinkgoT().Context(), "test-taskqueue")
Expect(err).To(MatchError(ContainSubstring("creating Task service")))
Expect(err).ToNot(MatchError(ContainSubstring("not exported")))
})
})
Describe("unstarted manager", func() {
It("enables a taskqueue plugin on a manager that was never started", func() {
// CLI commands (navidrome plugin enable) use the manager without calling Start
Expect(manager.EnablePlugin(GinkgoT().Context(), "test-taskqueue")).To(Succeed())
DeferCleanup(func() { _ = manager.unloadPlugin("test-taskqueue") })
})
})
})