diff --git a/go_backend/extension_availability_verification_test.go b/go_backend/extension_availability_verification_test.go index a2acd576..a89f6744 100644 --- a/go_backend/extension_availability_verification_test.go +++ b/go_backend/extension_availability_verification_test.go @@ -3,6 +3,7 @@ package gobackend import ( "strings" "testing" + "time" "github.com/dop251/goja" ) @@ -55,3 +56,74 @@ func TestAvailabilityDoesNotPromoteUntrustedOrStaleVerification(t *testing.T) { t.Fatalf("untrusted exception inherited a previous challenge: %v", err) } } + +func TestExtensionVerificationErrorsRequireOwnedPendingChallenge(t *testing.T) { + for _, tc := range []struct { + name string + message string + pendingOwner string + challengeAge time.Duration + authURL string + wantChallenge bool + }{ + {"fresh", "VERIFY_REQUIRED", "coverage-ext", 0, "https://example.test/verify", true}, + {"missing", "VERIFY_REQUIRED", "", 0, "", false}, + {"other-provider", "VERIFY_REQUIRED", "other-provider", 0, "https://example.test/verify", false}, + {"expired", "VERIFY_REQUIRED", "coverage-ext", pendingAuthRequestTTL + time.Second, "https://example.test/verify", false}, + {"future", "VERIFY_REQUIRED", "coverage-ext", -time.Minute, "https://example.test/verify", false}, + {"missing-url", "VERIFY_REQUIRED", "coverage-ext", 0, "", false}, + {"network-error", "network timeout", "coverage-ext", 0, "https://example.test/verify", false}, + {"provider-auth", "PROVIDER_AUTH_FAILED: VERIFY_REQUIRED", "coverage-ext", 0, "https://example.test/verify", false}, + {"http-status", "HTTP 401: VERIFY_REQUIRED", "coverage-ext", 0, "https://example.test/verify", false}, + {"cancelled", "cancelled: VERIFY_REQUIRED", "coverage-ext", 0, "https://example.test/verify", false}, + {"throwing-getter", "ordinary failure", "coverage-ext", 0, "https://example.test/verify", false}, + } { + t.Run(tc.name, func(t *testing.T) { + ext := newTestLoadedExtension(t, ExtensionTypeMetadataProvider, ExtensionTypeDownloadProvider) + t.Cleanup(func() { + ClearPendingAuthRequest(ext.ID) + ClearPendingAuthRequest(tc.pendingOwner) + teardownExtension(ext) + }) + if err := ext.ensureRuntimeReady(); err != nil { + t.Fatal(err) + } + if err := ext.VM.Set("fixtureMessage", tc.message); err != nil { + t.Fatal(err) + } + if _, err := ext.VM.RunString(`extension.searchTracks = extension.checkAvailability = function() { throw new Error(fixtureMessage); };`); err != nil { + t.Fatal(err) + } + if tc.name == "throwing-getter" { + if _, err := ext.VM.RunString(`extension.searchTracks = extension.checkAvailability = function() { + throw {toString: function() { return "ordinary failure"; }, get message() { throw new Error("broken getter"); }}; + };`); err != nil { + t.Fatal(err) + } + } + if tc.pendingOwner != "" { + if err := registerPendingAuthRequest(&PendingAuthRequest{ + ExtensionID: tc.pendingOwner, + AuthURL: tc.authURL, + CreatedAt: time.Now().Add(-tc.challengeAge), + }); err != nil { + t.Fatal(err) + } + } + provider := newExtensionProviderWrapper(ext) + _, searchErr := provider.SearchTracks("Song Artist", 1) + _, availabilityErr := provider.CheckAvailabilityForItemID("", "Song", "Artist", "", "", "", "", 180000, "") + for _, err := range []error{searchErr, availabilityErr} { + if err == nil { + t.Fatal("extension error was lost") + } + if got := classifyDownloadErrorType(err.Error()) == "verification_required"; got != tc.wantChallenge { + t.Fatalf("verification=%v, want %v: %v", got, tc.wantChallenge, err) + } + } + if tc.pendingOwner == "" && GetPendingAuthRequest(ext.ID) != nil { + t.Fatal("error classification created a challenge") + } + }) + } +} diff --git a/go_backend/extension_fallback.go b/go_backend/extension_fallback.go index 87f27962..660971ce 100644 --- a/go_backend/extension_fallback.go +++ b/go_backend/extension_fallback.go @@ -11,9 +11,9 @@ import ( // output path, reports progress, and on success assembles the full // DownloadResponse (overlay, request fallbacks, optional title/artist/composer // fallback, metadata embed, ISRC index). On failure it writes into -// lastErr/lastErrType/lastRetryAfterSeconds exactly as the inline code did -// (leaving them untouched when neither branch applies) so callers can keep -// their own verification_required/stop-fallback handling and error messages. +// lastErr/lastErrType/lastRetryAfterSeconds for the current attempt so callers +// can handle verification_required/stop-fallback without inheriting another +// provider's error or retry delay. // cancelledOuter true means the caller must return (nil, ErrDownloadCancelled). func attemptExtensionDownload( req DownloadRequest, @@ -26,6 +26,9 @@ func attemptExtensionDownload( lastErrType *string, lastRetryAfterSeconds *int, ) (resp *DownloadResponse, cancelledOuter bool) { + *lastErr = nil + *lastErrType = "" + *lastRetryAfterSeconds = 0 resolvedQuality, qualityErr := resolveExtensionDownloadQuality( quality, requestedQualityManifest(req, getExtensionManager()), ext.Manifest, ) @@ -176,11 +179,11 @@ func attemptExtensionDownload( } *lastErr = err *lastErrType = "" - } else if result != nil && result.ErrorMessage != "" { - *lastErr = fmt.Errorf("%s", result.ErrorMessage) - *lastErrType = normalizeExtensionDownloadErrorType(result.ErrorType, result.ErrorMessage) + } else if result != nil { + *lastErr = errors.New(firstNonEmptyTrimmed(result.ErrorMessage, "extension download failed without an error message")) + *lastErrType = firstNonEmptyTrimmed(normalizeExtensionDownloadErrorType(result.ErrorType, result.ErrorMessage), "extension_error") *lastRetryAfterSeconds = result.RetryAfterSeconds - } else if result == nil { + } else { *lastErr = fmt.Errorf("extension returned no download result") *lastErrType = "extension_error" } @@ -434,6 +437,7 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro var lastErr error var lastErrType string + var lastErrorService string var lastRetryAfterSeconds int var stopProviderFallback bool var sourceExtensionLocked bool @@ -599,6 +603,7 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro if resp != nil { return resp, nil } + lastErrorService = req.Source GoLog("[DownloadWithExtensionFallback] Source extension %s failed: %v\n", req.Source, lastErr) sourceErrType := lastErrType @@ -691,10 +696,13 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro } terminalAvailability := shouldStopProviderFallback(availability) if err != nil || !availability.Available { - GoLog("[DownloadWithExtensionFallback] %s: not available\n", providerID) if err != nil { lastErr = err - if strings.EqualFold(classifyDownloadErrorType(err.Error()), "verification_required") { + lastErrType = classifyDownloadErrorType(err.Error()) + lastErrorService = providerID + lastRetryAfterSeconds = 0 + GoLog("[DownloadWithExtensionFallback] %s availability failed: %v\n", providerID, err) + if strings.EqualFold(lastErrType, "verification_required") { GoLog("[DownloadWithExtensionFallback] %s requires verification (availability); pausing fallback to open the challenge\n", providerID) cachePreparedDownloadRequest(preparationKey, req) return &DownloadResponse{ @@ -704,6 +712,8 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro Service: providerID, }, nil } + } else { + GoLog("[DownloadWithExtensionFallback] %s: not available\n", providerID) } if terminalAvailability { GoLog("[DownloadWithExtensionFallback] %s requested skip_fallback after availability check\n", providerID) @@ -721,6 +731,7 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro if resp != nil { return resp, nil } + lastErrorService = providerID GoLog("[DownloadWithExtensionFallback] %s failed: %v\n", providerID, lastErr) if lastErr != nil { @@ -766,6 +777,7 @@ func DownloadWithExtensionFallback(req DownloadRequest) (*DownloadResponse, erro Error: "All providers failed. Last error: " + lastErr.Error(), ErrorType: errorType, RetryAfterSeconds: lastRetryAfterSeconds, + Service: lastErrorService, }, nil } diff --git a/go_backend/extension_fallback_verification_test.go b/go_backend/extension_fallback_verification_test.go new file mode 100644 index 00000000..667997d5 --- /dev/null +++ b/go_backend/extension_fallback_verification_test.go @@ -0,0 +1,136 @@ +package gobackend + +import ( + "fmt" + "io" + "net/http" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" +) + +func TestFallbackKeepsPendingVerificationOwnership(t *testing.T) { + for _, mode := range []string{"fresh-response", "saved-challenge", "network-error", "download-error", "empty-download-error"} { + t.Run(mode, func(t *testing.T) { + primary := newTestLoadedExtension(t, ExtensionTypeDownloadProvider) + primary.ID, primary.Manifest.Name = "primary-provider", "primary-provider" + secondary := newTestLoadedExtension(t, ExtensionTypeMetadataProvider, ExtensionTypeDownloadProvider) + secondary.ID, secondary.Manifest.Name = "secondary-provider", "secondary-provider" + secondary.Manifest.Permissions.Storage = true + secondary.Manifest.SignedSession = &SignedSessionConfig{ + Namespace: "fixture-session", BaseURL: "https://auth.example.test", + } + last := newTestLoadedExtension(t, ExtensionTypeDownloadProvider) + last.ID, last.Manifest.Name = "last-provider", "last-provider" + for ext, script := range map[*loadedExtension]string{ + primary: `registerExtension({ + checkAvailability: function() { return {available: true, track_id: "source-track"}; }, + download: function() { return {success: false, error_type: "api_error", error_message: "primary request failed", retry_after_seconds: 37}; } + });`, + secondary: `var savedChallenge; + function queryCatalog() { + var response = session.signedFetch("GET", "/catalog"); + if (response.needsVerification) { + savedChallenge = new Error("VERIFY_REQUIRED"); + throw savedChallenge; + } + throw new Error("fixture expected a challenge"); + } + registerExtension({ + searchTracks: queryCatalog, + checkAvailability: function() { + if (fixtureMode === "network-error") throw new Error("lookup network timeout"); + if (fixtureMode === "download-error" || fixtureMode === "empty-download-error") return {available: true, track_id: "matched-track"}; + if (fixtureMode === "saved-challenge") throw savedChallenge; + return queryCatalog(); + }, + download: function() { + if (fixtureMode === "download-error") throw new Error("download network timeout"); + if (fixtureMode === "empty-download-error") return {success: false}; + throw new Error("verification must finish before download"); + } + });`, + last: `registerExtension({checkAvailability: function() { recordLastProvider(); return {available: false}; }});`, + } { + if err := os.WriteFile(filepath.Join(ext.SourceDir, "index.js"), fmt.Appendf(nil, "var fixtureMode = %q;\n%s", mode, script), 0600); err != nil { + t.Fatal(err) + } + } + manager := getExtensionManager() + manager.mu.Lock() + previousExtensions := manager.extensions + manager.extensions = map[string]*loadedExtension{primary.ID: primary, secondary.ID: secondary, last.ID: last} + manager.mu.Unlock() + previousPriority, previousFallback := GetProviderPriority(), GetExtensionFallbackProviderIDs() + SetProviderPriority([]string{secondary.ID, last.ID, primary.ID}) + SetExtensionFallbackProviderIDs(nil) + t.Cleanup(func() { + for _, ext := range []*loadedExtension{primary, secondary, last} { + ClearPendingAuthRequest(ext.ID) + teardownExtension(ext) + } + manager.mu.Lock() + manager.extensions = previousExtensions + manager.mu.Unlock() + SetProviderPriority(previousPriority) + SetExtensionFallbackProviderIDs(previousFallback) + resetPreparedDownloadRequestCacheForTest() + }) + if err := secondary.ensureRuntimeReady(); err != nil { + t.Fatal(err) + } + var bootstrapCalls atomic.Int32 + secondary.runtime.httpClient = &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) { + bootstrapCalls.Add(1) + return &http.Response{ + StatusCode: http.StatusOK, Header: make(http.Header), Request: req, + Body: io.NopCloser(strings.NewReader(`{"auth_url":"https://auth.example.test/verify"}`)), + }, nil + })} + expectsVerification := mode == "fresh-response" || mode == "saved-challenge" + if expectsVerification { + _, err := newExtensionProviderWrapper(secondary).SearchTracks("Song Artist", 1) + if err == nil || GetPendingAuthRequest(secondary.ID) == nil { + t.Fatalf("metadata lookup did not create a pending challenge: %v", err) + } + } + if err := last.ensureRuntimeReady(); err != nil { + t.Fatal(err) + } + var laterCalls atomic.Int32 + if err := last.VM.Set("recordLastProvider", func() { laterCalls.Add(1) }); err != nil { + t.Fatal(err) + } + response, err := DownloadWithExtensionFallback(DownloadRequest{ + Service: primary.ID, Source: primary.ID, ItemID: "fallback-" + mode, + SpotifyID: "source-track", TrackName: "Song", ArtistName: "Artist", AlbumName: "Album", + ISRC: "USABC2600001", ReleaseDate: "2026-01-01", Quality: "lossless", + OutputDir: t.TempDir(), FilenameFormat: "{title}", UseFallback: true, + }) + if err != nil || response == nil { + t.Fatalf("fallback response=%+v error=%v", response, err) + } + wantType := "verification_required" + switch mode { + case "network-error": + wantType = "network" + case "download-error": + wantType = "script_error" + case "empty-download-error": + wantType = "extension_error" + } + if response.ErrorType != wantType || response.Service != secondary.ID || response.RetryAfterSeconds != 0 { + t.Fatalf("failure lost its type, owner, or retry delay: %+v", response) + } + if expectsVerification { + if laterCalls.Load() != 0 || bootstrapCalls.Load() != 1 { + t.Fatalf("pending challenge was skipped or recreated: later=%d bootstrap=%d", laterCalls.Load(), bootstrapCalls.Load()) + } + } else if laterCalls.Load() != 1 { + t.Fatal("ordinary lookup failures must allow the next provider") + } + }) + } +} diff --git a/go_backend/extension_provider_wrapper.go b/go_backend/extension_provider_wrapper.go index e015b30a..a73fbca3 100644 --- a/go_backend/extension_provider_wrapper.go +++ b/go_backend/extension_provider_wrapper.go @@ -105,7 +105,11 @@ func callExtension[T any](p *extensionProviderWrapper, opts extCallOpts, parse f jsStartedAt := time.Now() result, err := runGojaCallWithTimeoutContextAndRecover(ctx, p.vm, func() (goja.Value, error) { - return opts.invoke(p.vm) + result, err := opts.invoke(p.vm) + if err != nil { + err = p.normalizePendingVerificationError(err) + } + return result, err }, opts.timeout) perf.recordJS(time.Since(jsStartedAt)) perf.recordPayload(result) @@ -143,6 +147,40 @@ func callExtension[T any](p *extensionProviderWrapper, opts extCallOpts, parse f return parse(perf, result) } +func (p *extensionProviderWrapper) normalizePendingVerificationError(err error) error { + var exception *goja.Exception + if !errors.As(err, &exception) { + return err + } + // A script may rethrow an earlier challenge after the per-call runtime + // marker was cleared. Only trust an existing, fresh challenge for this + // extension; the exception alone must not start verification. + pending := GetPendingAuthRequest(p.extension.ID) + if pending == nil || pending.ExtensionID != p.extension.ID || strings.TrimSpace(pending.AuthURL) == "" { + return err + } + if age := time.Since(pending.CreatedAt); age < 0 || age >= pendingAuthRequestTTL { + return err + } + value := exception.Value() + if gojaValueIsEmpty(value) { + return err + } + var message string + if extractionErr := p.vm.Try(func() { + if object, ok := value.(*goja.Object); ok { + if field := object.Get("message"); !gojaValueIsEmpty(field) { + message = field.String() + } + } else { + message = value.String() + } + }); extractionErr != nil || strings.TrimSpace(message) != "VERIFY_REQUIRED" { + return err + } + return fmt.Errorf("verification_required: extension '%s' needs signed-session verification: %w", p.extension.ID, err) +} + func invokeExtensionMethod(vm *goja.Runtime, method string, args ...any) (goja.Value, error) { extensionValue := vm.Get("extension") if gojaValueIsEmpty(extensionValue) {