Commit Graph

5 Commits

Author SHA1 Message Date
Deluan Quintão
205c85da55 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.
2026-07-10 19:15:49 -04:00
Deluan
2a43c4683e chore: go fix
Signed-off-by: Deluan <deluan@navidrome.org>
2026-05-28 22:13:05 -03:00
Deluan Quintão
8f0b4930ff refactor(conf): replace eager dir creation with lazy Dir type (#5495)
* feat(conf): add Dir type with lazy directory creation

Introduces the Dir type that wraps a directory path string and defers
os.MkdirAll until the first call to Path() or MustPath(), using sync.Once
to ensure the creation happens exactly once. Implements fmt.Stringer,
encoding.TextMarshaler, and encoding.TextUnmarshaler for config integration.
Includes Ginkgo/Gomega tests covering all methods and error paths.

* refactor(conf): replace eager dir creation with lazy Dir type

Change DataFolder, CacheFolder, Plugins.Folder, and Backup.Path from
string to Dir. Remove all os.MkdirAll calls from Load() so directories
are created lazily on first Path()/MustPath() call. Artwork folder
creation was already handled at point-of-use in image_upload.go.

Add SnapshotConfig() to conf package for safe test config save/restore
that avoids copying sync.Once inside Dir fields. Fix copy-lock vet
warning in nativeapi/config.go by marshalling pointer instead of value.

* refactor(conf): migrate tests and db init to lazy Dir type

Update all test files to use conf.NewDir() for Dir field assignments.
Ensure DataFolder is created lazily when the database is first opened
in db.Db(). Remove eager directory creation from conf.Load() tests.

* fix(conf): address review findings for Dir type

- Use os.ModePerm for DataFolder/CacheFolder (was 0700, should match
  original behavior). Add NewDirWithPerm for PluginsFolder (0700).
- Use Path() instead of MustPath() in db.Prune() to avoid logFatal
  from background cron job.
- Panic on marshal/unmarshal errors in SnapshotConfig (test helper).
- Clean up redundant String()/MustPath() calls in plugin manager.
- Remove dead code in dir_test.go.

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(conf): add GoString to Dir for clean config dump output

Implement fmt.GoStringer on Dir so pretty.Sprintf shows the path
string instead of internal struct fields (sync.Once, perm, err).
Also add TODO comment to configtest about removing the indirection.

* fix(dir): improve error logging in MustPath method

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(tests): remove redundant tests for unwritable DataFolder and CacheFolder

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(conf): address PR review feedback

- Ensure Plugins.Folder always uses 0700, even when user-configured
  (previously only the derived default got restrictive permissions).
- Create LogFile parent directory before opening, so LogFile paths
  inside a not-yet-created DataFolder work correctly.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-05-13 17:44:22 -03:00
Deluan
3cd5d16b0a chore: upgrade golangci-lint to 2.11 and fix lint issues
Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-06 19:23:47 -05:00
Deluan Quintão
668869b6c7 feat(plugins): add TaskQueue host service for persistent background task queues (#5116)
* feat(plugins): define TaskQueue host service interface

Add the TaskQueueService interface with CreateQueue, Enqueue,
GetTaskStatus, and CancelTask methods plus QueueConfig struct.

* feat(plugins): define TaskWorker capability for task execution callbacks

* feat(plugins): add taskqueue permission to manifest schema

Add TaskQueuePermission with maxConcurrency option.

* feat(plugins): implement TaskQueue service with SQLite persistence and workers

Per-plugin SQLite database with queues and tasks tables. Worker goroutines
dequeue tasks and invoke nd_task_execute callback. Exponential backoff
retries, rate limiting via delayMs, automatic cleanup of terminal tasks.

* feat(plugins): require TaskWorker capability for taskqueue permission

* feat(plugins): register TaskQueue host service in manager

* feat(plugins): add test-taskqueue plugin for integration testing

* feat(plugins): add integration tests for TaskQueue host service

* docs: document TaskQueue module for persistent task queues

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(plugins): harden TaskQueue host service with validation and safety improvements

Add input validation (queue name length, payload size limits), extract
status string constants to eliminate raw SQL literals, make CreateQueue
idempotent via upsert for crash recovery, fix RetentionMs default check
for negative values, cap exponential backoff at 1 hour to prevent
overflow, and replace manual mutex-based delay enforcement with
rate.Limiter from golang.org/x/time/rate for correct concurrent worker
serialization.

* refactor(plugins): remove capability check for TaskWorker in TaskQueue host service

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(plugins): use context-aware database execution in TaskQueue host service

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): streamline task queue configuration and error handling

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): increase maxConcurrency for task queue and handle budget exhaustion

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): simplify goroutine management in task queue service

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): update TaskWorker interface to return status messages and refactor task queue service

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(plugins): add ClearQueue function to remove pending tasks from a specified queue

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(plugins): use migrateDB for task queue schema and fix constant name collision

Replaced the raw db.Exec call in createTaskQueueSchema with migrateDB,
matching the pattern used by createKVStoreSchema. This enables version-tracked
schema migrations via SQLite's PRAGMA user_version, allowing future schema
changes to be appended incrementally. Also renamed cleanupInterval to
taskCleanupInterval to resolve a redeclaration conflict with host_kvstore.go.

* regenerate PDKs

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-03 13:48:49 -05:00