diff --git a/plugins/host_artwork_test.go b/plugins/host_artwork_test.go index 22915ca3f..5e3c54a80 100644 --- a/plugins/host_artwork_test.go +++ b/plugins/host_artwork_test.go @@ -111,7 +111,7 @@ var _ = Describe("ArtworkService", Ordered, func() { p := manager.plugins["test-artwork"] manager.mu.RUnlock() - instance, err := p.instance() + instance, err := p.instance(ctx) if err != nil { return "", err } diff --git a/plugins/host_cache_test.go b/plugins/host_cache_test.go index 298aa7e97..ec225c1c2 100644 --- a/plugins/host_cache_test.go +++ b/plugins/host_cache_test.go @@ -408,7 +408,7 @@ var _ = Describe("CacheService Integration", Ordered, func() { p := manager.plugins["test-cache-plugin"] manager.mu.RUnlock() - instance, err := p.instance() + instance, err := p.instance(ctx) if err != nil { return nil, err } diff --git a/plugins/host_kvstore_test.go b/plugins/host_kvstore_test.go index f8eae0395..3e2cbd01a 100644 --- a/plugins/host_kvstore_test.go +++ b/plugins/host_kvstore_test.go @@ -434,7 +434,7 @@ var _ = Describe("KVStoreService Integration", Ordered, func() { p := manager.plugins["test-kvstore"] manager.mu.RUnlock() - instance, err := p.instance() + instance, err := p.instance(ctx) if err != nil { return nil, err } diff --git a/plugins/host_library_test.go b/plugins/host_library_test.go index 3054b524b..5eb896e09 100644 --- a/plugins/host_library_test.go +++ b/plugins/host_library_test.go @@ -363,7 +363,7 @@ var _ = Describe("LibraryService Integration", Ordered, func() { p := manager.plugins["test-library"] manager.mu.RUnlock() - instance, err := p.instance() + instance, err := p.instance(ctx) if err != nil { return nil, err } diff --git a/plugins/host_scheduler_test.go b/plugins/host_scheduler_test.go index da4d04dbc..0233ce781 100644 --- a/plugins/host_scheduler_test.go +++ b/plugins/host_scheduler_test.go @@ -273,7 +273,7 @@ var _ = Describe("SchedulerService", Ordered, func() { Expect(testService.GetScheduleCount()).To(Equal(1)) // Create a plugin instance - instance, err := plugin.instance() + instance, err := plugin.instance(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) defer instance.Close(GinkgoT().Context()) diff --git a/plugins/host_subsonicapi_test.go b/plugins/host_subsonicapi_test.go index c197014aa..1843c4e49 100644 --- a/plugins/host_subsonicapi_test.go +++ b/plugins/host_subsonicapi_test.go @@ -130,7 +130,7 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() { }) It("successfully calls the ping endpoint", func() { - instance, err := plugin.instance() + instance, err := plugin.instance(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) defer instance.Close(GinkgoT().Context()) @@ -149,7 +149,7 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() { }) It("adds required parameters (c, f, v) to the request", func() { - instance, err := plugin.instance() + instance, err := plugin.instance(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) defer instance.Close(GinkgoT().Context()) @@ -166,7 +166,7 @@ var _ = Describe("SubsonicAPI Host Function", Ordered, func() { }) It("returns error when username is missing", func() { - instance, err := plugin.instance() + instance, err := plugin.instance(GinkgoT().Context()) Expect(err).ToNot(HaveOccurred()) defer instance.Close(GinkgoT().Context()) diff --git a/plugins/manager_call.go b/plugins/manager_call.go index 312a9a5d3..77e5b37ac 100644 --- a/plugins/manager_call.go +++ b/plugins/manager_call.go @@ -27,13 +27,15 @@ func callPluginFunctionNoOutput[I any](ctx context.Context, plugin *plugin, func // callPluginFunction is a helper to call a plugin function with input and output types. // It handles JSON marshalling/unmarshalling and error checking. +// The context is used for cancellation - if cancelled during the call, the plugin +// instance will be terminated and context.Canceled or context.DeadlineExceeded will be returned. func callPluginFunction[I any, O any](ctx context.Context, plugin *plugin, funcName string, input I) (O, error) { start := time.Now() var result O - // Create plugin instance - p, err := plugin.instance() + // Create plugin instance with context for cancellation support + p, err := plugin.instance(ctx) if err != nil { return result, fmt.Errorf("failed to create plugin: %w", err) } @@ -50,8 +52,13 @@ func callPluginFunction[I any, O any](ctx context.Context, plugin *plugin, funcN } startCall := time.Now() - exit, output, err := p.Call(funcName, inputBytes) + exit, output, err := p.CallWithContext(ctx, funcName, inputBytes) if err != nil { + // If context was cancelled, return that error instead of the plugin error + if ctx.Err() != nil { + log.Debug(ctx, "Plugin call cancelled", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall)) + return result, ctx.Err() + } log.Trace(ctx, "Plugin call failed", "plugin", plugin.name, "function", funcName, "pluginDuration", time.Since(startCall), "navidromeDuration", startCall.Sub(start), err) return result, fmt.Errorf("plugin call failed: %w", err) } diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go index 4c4125c45..2291b1f88 100644 --- a/plugins/manager_loader.go +++ b/plugins/manager_loader.go @@ -269,8 +269,10 @@ func (m *Manager) loadPluginWithConfig(name, ndpPath, configJSON string) error { // Compile the plugin with all host functions extismConfig := extism.PluginConfig{ - EnableWasi: true, - RuntimeConfig: wazero.NewRuntimeConfig().WithCompilationCache(m.cache), + EnableWasi: true, + RuntimeConfig: wazero.NewRuntimeConfig(). + WithCompilationCache(m.cache). + WithCloseOnContextDone(true), } compiled, err := extism.NewCompiledPlugin(m.ctx, pluginManifest, extismConfig, hostFunctions) if err != nil { diff --git a/plugins/manager_plugin.go b/plugins/manager_plugin.go index e719ee6a0..44751e178 100644 --- a/plugins/manager_plugin.go +++ b/plugins/manager_plugin.go @@ -20,8 +20,11 @@ type plugin struct { closers []io.Closer // Cleanup functions to call on unload } -func (p *plugin) instance() (*extism.Plugin, error) { - instance, err := p.compiled.Instance(context.Background(), extism.PluginInstanceConfig{ +// instance creates a new plugin instance for the given context. +// The context is used for cancellation - if cancelled during a call, +// the module will be terminated and the instance becomes unusable. +func (p *plugin) instance(ctx context.Context) (*extism.Plugin, error) { + instance, err := p.compiled.Instance(ctx, extism.PluginInstanceConfig{ ModuleConfig: wazero.NewModuleConfig().WithSysWalltime().WithRandSource(rand.Reader), }) if err != nil { diff --git a/plugins/metadata_agent_test.go b/plugins/metadata_agent_test.go index 546661d98..96957b71a 100644 --- a/plugins/metadata_agent_test.go +++ b/plugins/metadata_agent_test.go @@ -1,8 +1,6 @@ package plugins import ( - "context" - "github.com/navidrome/navidrome/core/agents" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -10,10 +8,8 @@ import ( var _ = Describe("MetadataAgent", Ordered, func() { var agent agents.Interface - var ctx context.Context BeforeAll(func() { - ctx = GinkgoT().Context() // Load the agent via shared manager var ok bool agent, ok = testManager.LoadMediaAgent("test-metadata-agent") @@ -29,7 +25,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetArtistMBID", func() { It("returns the MBID from the plugin", func() { retriever := agent.(agents.ArtistMBIDRetriever) - mbid, err := retriever.GetArtistMBID(ctx, "artist-1", "The Beatles") + mbid, err := retriever.GetArtistMBID(GinkgoT().Context(), "artist-1", "The Beatles") Expect(err).ToNot(HaveOccurred()) Expect(mbid).To(Equal("test-mbid-The Beatles")) }) @@ -38,7 +34,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetArtistURL", func() { It("returns the URL from the plugin", func() { retriever := agent.(agents.ArtistURLRetriever) - url, err := retriever.GetArtistURL(ctx, "artist-1", "The Beatles", "some-mbid") + url, err := retriever.GetArtistURL(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid") Expect(err).ToNot(HaveOccurred()) Expect(url).To(Equal("https://test.example.com/artist/The Beatles")) }) @@ -47,7 +43,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetArtistBiography", func() { It("returns the biography from the plugin", func() { retriever := agent.(agents.ArtistBiographyRetriever) - bio, err := retriever.GetArtistBiography(ctx, "artist-1", "The Beatles", "some-mbid") + bio, err := retriever.GetArtistBiography(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid") Expect(err).ToNot(HaveOccurred()) Expect(bio).To(Equal("Biography for The Beatles")) }) @@ -56,7 +52,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetArtistImages", func() { It("returns images from the plugin", func() { retriever := agent.(agents.ArtistImageRetriever) - images, err := retriever.GetArtistImages(ctx, "artist-1", "The Beatles", "some-mbid") + images, err := retriever.GetArtistImages(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid") Expect(err).ToNot(HaveOccurred()) Expect(images).To(HaveLen(2)) Expect(images[0].URL).To(Equal("https://test.example.com/images/The Beatles/large.jpg")) @@ -69,7 +65,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetSimilarArtists", func() { It("returns similar artists from the plugin", func() { retriever := agent.(agents.ArtistSimilarRetriever) - artists, err := retriever.GetSimilarArtists(ctx, "artist-1", "The Beatles", "some-mbid", 3) + artists, err := retriever.GetSimilarArtists(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid", 3) Expect(err).ToNot(HaveOccurred()) Expect(artists).To(HaveLen(3)) Expect(artists[0].Name).To(Equal("The Beatles Similar A")) @@ -81,7 +77,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetArtistTopSongs", func() { It("returns top songs from the plugin", func() { retriever := agent.(agents.ArtistTopSongsRetriever) - songs, err := retriever.GetArtistTopSongs(ctx, "artist-1", "The Beatles", "some-mbid", 3) + songs, err := retriever.GetArtistTopSongs(GinkgoT().Context(), "artist-1", "The Beatles", "some-mbid", 3) Expect(err).ToNot(HaveOccurred()) Expect(songs).To(HaveLen(3)) Expect(songs[0].Name).To(Equal("The Beatles Song 1")) @@ -93,7 +89,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetAlbumInfo", func() { It("returns album info from the plugin", func() { retriever := agent.(agents.AlbumInfoRetriever) - info, err := retriever.GetAlbumInfo(ctx, "Abbey Road", "The Beatles", "album-mbid") + info, err := retriever.GetAlbumInfo(GinkgoT().Context(), "Abbey Road", "The Beatles", "album-mbid") Expect(err).ToNot(HaveOccurred()) Expect(info.Name).To(Equal("Abbey Road")) Expect(info.MBID).To(Equal("test-album-mbid-Abbey Road")) @@ -105,7 +101,7 @@ var _ = Describe("MetadataAgent", Ordered, func() { Describe("GetAlbumImages", func() { It("returns album images from the plugin", func() { retriever := agent.(agents.AlbumImageRetriever) - images, err := retriever.GetAlbumImages(ctx, "Abbey Road", "The Beatles", "album-mbid") + images, err := retriever.GetAlbumImages(GinkgoT().Context(), "Abbey Road", "The Beatles", "album-mbid") Expect(err).ToNot(HaveOccurred()) Expect(images).To(HaveLen(1)) Expect(images[0].URL).To(Equal("https://test.example.com/albums/Abbey Road/cover.jpg")) @@ -119,12 +115,9 @@ var _ = Describe("MetadataAgent error handling", Ordered, func() { var ( errorManager *Manager errorAgent agents.Interface - ctx context.Context ) BeforeAll(func() { - ctx = GinkgoT().Context() - // Create manager with error injection config errorManager, _ = createTestManager(map[string]map[string]string{ "test-metadata-agent": { @@ -140,56 +133,56 @@ var _ = Describe("MetadataAgent error handling", Ordered, func() { It("returns error from GetArtistMBID", func() { retriever := errorAgent.(agents.ArtistMBIDRetriever) - _, err := retriever.GetArtistMBID(ctx, "artist-1", "Test") + _, err := retriever.GetArtistMBID(GinkgoT().Context(), "artist-1", "Test") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetArtistURL", func() { retriever := errorAgent.(agents.ArtistURLRetriever) - _, err := retriever.GetArtistURL(ctx, "artist-1", "Test", "mbid") + _, err := retriever.GetArtistURL(GinkgoT().Context(), "artist-1", "Test", "mbid") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetArtistBiography", func() { retriever := errorAgent.(agents.ArtistBiographyRetriever) - _, err := retriever.GetArtistBiography(ctx, "artist-1", "Test", "mbid") + _, err := retriever.GetArtistBiography(GinkgoT().Context(), "artist-1", "Test", "mbid") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetArtistImages", func() { retriever := errorAgent.(agents.ArtistImageRetriever) - _, err := retriever.GetArtistImages(ctx, "artist-1", "Test", "mbid") + _, err := retriever.GetArtistImages(GinkgoT().Context(), "artist-1", "Test", "mbid") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetSimilarArtists", func() { retriever := errorAgent.(agents.ArtistSimilarRetriever) - _, err := retriever.GetSimilarArtists(ctx, "artist-1", "Test", "mbid", 5) + _, err := retriever.GetSimilarArtists(GinkgoT().Context(), "artist-1", "Test", "mbid", 5) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetArtistTopSongs", func() { retriever := errorAgent.(agents.ArtistTopSongsRetriever) - _, err := retriever.GetArtistTopSongs(ctx, "artist-1", "Test", "mbid", 5) + _, err := retriever.GetArtistTopSongs(GinkgoT().Context(), "artist-1", "Test", "mbid", 5) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetAlbumInfo", func() { retriever := errorAgent.(agents.AlbumInfoRetriever) - _, err := retriever.GetAlbumInfo(ctx, "Album", "Artist", "mbid") + _, err := retriever.GetAlbumInfo(GinkgoT().Context(), "Album", "Artist", "mbid") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) It("returns error from GetAlbumImages", func() { retriever := errorAgent.(agents.AlbumImageRetriever) - _, err := retriever.GetAlbumImages(ctx, "Album", "Artist", "mbid") + _, err := retriever.GetAlbumImages(GinkgoT().Context(), "Album", "Artist", "mbid") Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) diff --git a/plugins/scrobbler_adapter_test.go b/plugins/scrobbler_adapter_test.go index 14bffb4aa..625e15036 100644 --- a/plugins/scrobbler_adapter_test.go +++ b/plugins/scrobbler_adapter_test.go @@ -14,18 +14,20 @@ import ( . "github.com/onsi/gomega" ) +// ctxWithUser returns a fresh context with the test user. +// Must be called within each test, not in BeforeAll, because the context +// from BeforeAll gets cancelled before tests run. +func ctxWithUser() context.Context { + return request.WithUser(GinkgoT().Context(), model.User{ID: "user-1", UserName: "testuser"}) +} + var _ = Describe("ScrobblerPlugin", Ordered, func() { var ( scrobblerManager *Manager s scrobbler.Scrobbler - ctx context.Context ) BeforeAll(func() { - ctx = GinkgoT().Context() - // Add user to context for username extraction - ctx = request.WithUser(ctx, model.User{ID: "user-1", UserName: "testuser"}) - // Load the scrobbler via a new manager with the test-scrobbler plugin scrobblerManager, _ = createTestManagerWithPlugins(nil, "test-scrobbler"+PackageExtension) @@ -52,7 +54,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { Describe("IsAuthorized", func() { It("returns true when plugin is configured to authorize", func() { - result := s.IsAuthorized(ctx, "user-1") + result := s.IsAuthorized(ctxWithUser(), "user-1") Expect(result).To(BeTrue()) }) @@ -64,7 +66,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { sc, ok := manager.LoadScrobbler("test-scrobbler") Expect(ok).To(BeTrue()) - result := sc.IsAuthorized(ctx, "user-1") + result := sc.IsAuthorized(ctxWithUser(), "user-1") Expect(result).To(BeFalse()) }) }) @@ -82,7 +84,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { DiscNumber: 1, } - err := s.NowPlaying(ctx, "user-1", track, 30) + err := s.NowPlaying(ctxWithUser(), "user-1", track, 30) Expect(err).ToNot(HaveOccurred()) }) @@ -95,7 +97,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { Expect(ok).To(BeTrue()) track := &model.MediaFile{ID: "track-1", Title: "Test Song"} - err := sc.NowPlaying(ctx, "user-1", track, 30) + err := sc.NowPlaying(ctxWithUser(), "user-1", track, 30) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(scrobbler.ErrRetryLater)) }) @@ -117,7 +119,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { TimeStamp: time.Now(), } - err := s.Scrobble(ctx, "user-1", sc) + err := s.Scrobble(ctxWithUser(), "user-1", sc) Expect(err).ToNot(HaveOccurred()) }) @@ -133,7 +135,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { MediaFile: model.MediaFile{ID: "track-1", Title: "Test Song"}, TimeStamp: time.Now(), } - err := sc.Scrobble(ctx, "user-1", scrobble) + err := sc.Scrobble(ctxWithUser(), "user-1", scrobble) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(scrobbler.ErrNotAuthorized)) }) @@ -150,7 +152,7 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { MediaFile: model.MediaFile{ID: "track-1", Title: "Test Song"}, TimeStamp: time.Now(), } - err := sc.Scrobble(ctx, "user-1", scrobble) + err := sc.Scrobble(ctxWithUser(), "user-1", scrobble) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError(scrobbler.ErrUnrecoverable)) })