Recommendations endpoint publisher doesn't complete in the case of 429 error

[ccrama]

Love this library, thank you all your hard work!

I have noticed that, when I get a 429 error (rate limited) on the Recommendations endpoint, the sink subscriber doesn't get a value from the publisher and the logger doesn't seem to pick up on the error either. I have not seen this behavior on other endpoints for 5XX errors. I was able to track down the 429 using CharlesProxy.

Here's a snippet of code where I'm calling the recommendations endpoint:

        spotifyController.api.recommendations(TrackAttributes(seedArtists: type == .artist ? ["spotify:\(type.rawValue):\(id)"] : nil, seedTracks: type == .track ? ["spotify:\(type.rawValue):\(id)"] : nil), limit: 100, market: "US")
            .receive(on: RunLoop.main)
            .sink(
                receiveCompletion: { [weak self] completion in
                    print("This doesn't print on a 429 error")
                    guard let self = self else { return }
                    switch completion {
                    case .finished:
                        self.isLoading = false
                    case .failure(let error):
                        // This doesn't print either
                        print(error)
                        self.error = error
                    }
                },
                receiveValue: { [weak self] containers in
                    guard let self = self else { return }
                    self.tracks = containers.tracks.filter({ track in
                        track.id != nil && track.previewURL != nil
                    })
                }
            )
            .store(in: &cancellables)
    }

I have logging set to .trace, I only get the following output:

trace APIRequest : [SpotifyWebAPI] GET request to "https://api.spotify.com/v1/recommendations?limit=100&market=US&seed_tracks=1Mc6hi4oy0xYAcclSPoQF1"

Is this an issue with my implementation of the subscriber, or is there another issue at play? I am a bit worried that the subscriber never gets called, and this instance is never removed from cancellables.

Thank you!

[ccrama]

Update: I do get a result on other 400 errors. Here, I tested with an invalid seed_track to get a bad request

trace APIRequest : [SpotifyWebAPI] GET request to "https://api.spotify.com/v1/recommendations?limit=100&market=US&seed_tracks=INVALID_TRACK"
This doesn't print on a 429 error
SpotifyError(message: "invalid request", statusCode: 400)

[Peter-Schorn]

Try removing weak self.

[ccrama]

Thank you for the quick response!

A strong self doesn't fix the issue, unfortunately. Even in the case that self was nil, I would expect the subscriber would print the line that is above the guard let self {...}. I can also confirm my data struct (the wrapper for the Recommendations endpoint that is calling spotifyController.api.recommendations) is not being dereferenced and has a strong connection to the view on screen, and self wouldn't be nil here.

I do see the error printed from a 400 (bad request) using a weak self, and I still get no completion to the publisher when using strong self with a 429.

[ccrama]

I think I tracked this down to retryOnSpotifyErrors(), this publisher will retry a rate limited response if running in debug mode:

                #if DEBUG
                DebugHooks.receiveRateLimitedError.send(rateLimitedError)
                #endif

Spotify really hounds you on the Recommendations API. Since my app has not been granted an extension, any sort of rate limiting is set to ~12.5 hours on the first offense. Since I've been running my app in debug mode, I believe SpotifyAPI is trying to wait 12.5 hours to retry again. I currently am not rate limited and can't test this at the moment, but I think it would be useful to be able to toggle the retry mechanism off globally for a SpotifyAPI, and/or allow users to enable this when not in debug mode.

[Peter-Schorn]

#if DEBUG
DebugHooks.receiveRateLimitedError.send(rateLimitedError)
#endif

This specific line of code merely sends the RateLimitedError to a subject that is only subscribed to in unit tests, which ensure a rate limit error can be correctly decoded and handled by my library. Other than in the unit tests, this line of code has no effect.

Note that the rest of retryOnSpotifyErrors() is not wrapped in any conditional compilation block. The retry logic in this library always applies, and in the same way, regardless of whether the code is running in debug mode or not.

This library will retry a request up to three times depending on the error received.

Since my app has not been granted an extension, any sort of rate limiting is set to ~12.5 hours on the first offense.

Even in development mode, are you sure you have to wait that long before sending additional requests once you get rate-limited? Spotify's documentation says rate limits apply based on the number of calls you make in a rolling 30-second window, which would suggest that you shouldn't have to wait more than 30 seconds to retry the request. Try setting a breakpoint and examining the value of the retryAfter property of RateLimitedError (the number of seconds you must wait before you try the request again):

if let rateLimitedError = error as? RateLimitedError {
    #if DEBUG
    DebugHooks.receiveRateLimitedError.send(rateLimitedError)
    #endif
//            Swift.print("retryOnRateLimitedError: \(rateLimitedError)")
    let secondsDelay = (rateLimitedError.retryAfter ?? 3) + 1  // <--- set a breakpoint here and `po rateLimitedError.retryAfter`

    switch additionalRetries {
        case 3:
            return .seconds(secondsDelay)
        // Adding random delays improves the success rate
        // of concurrent requests. If all requests were
        // serialized, then we would never get a rate
        // limited error more than once per request in the
        // first place.
        case 2:
            var millisecondsDelay = secondsDelay * 1_000
            // + 0...5 seconds
            millisecondsDelay += Int.random(in: 0...5_000)
            return .milliseconds(millisecondsDelay)
        default /* 1 */:
            var millisecondsDelay = secondsDelay * 1000
            // + 5...10 seconds
            millisecondsDelay += Int.random(in: 5_000...10_000)
            return .milliseconds(millisecondsDelay)
    }
}

I was able to track down the 429 using CharlesProxy.

When you get a response with this status code, what is the value of the Retry-After header? Does it indicate you must wait ~12.5 hours before retrying the request?

Lastly, try adding the handleEvents operator right before your sink:

spotifyController.api.recommendations(TrackAttributes(seedArtists: type == .artist ? ["spotify:\(type.rawValue):\(id)"] : nil, seedTracks: type == .track ? ["spotify:\(type.rawValue):\(id)"] : nil), limit: 100, market: "US")
    .receive(on: RunLoop.main)
    .handleEvents(
        receiveSubscription: { subscription in
            print("receiveSubscription: \(subscription)")
        },
        receiveOutput: { output in
            print("receiveOutput: \(output)")
        },
        receiveCompletion: { completion in
            print("receiveCompletion: \(completion)")
        },
        receiveCancel: {
            print("receive cancel")
        },
        receiveRequest: { request in
            print("receiveRequest: \(request)")
        }
    )
    .sink(
        receiveCompletion: { [weak self] completion in
            print("This doesn't print on a 429 error")
            guard let self = self else { return }
            switch completion {
                case .finished:
                    self.isLoading = false
                case .failure(let error):
                    // This doesn't print either
                    print(error)
                    self.error = error
            }
        },
        receiveValue: { [weak self] containers in
            guard let self = self else { return }
            self.tracks = containers.tracks.filter({ track in
                track.id != nil && track.previewURL != nil
            })
        }
    )
    .store(in: &cancellables)

[ccrama]

Thank you for confirming the retry logic. I poked around the documentation and didn't see a mention of 429 errors or the retry mechanism, and I hadn't looked too deeply at the code yet. I'll open up a merge request to add some context around the retry mechanism, and add some documentation for the 429 error in SpotifyError.

When you get a response with this status code, what is the value of the Retry-After header? Does it indicate you must wait ~12.5 hours before retrying the request?

Yes, my retry-after was 48200, which is actually closer to 14 hours. It seems that Spotify protects their recommendations API more than their other APIs, likely to stop scraping for AI training purposes. I am not able to use those features of my application while I'm in the 429 period, and it has happened multiple times where I have to wait half a day to continue usage. I am carefully tracking my usage of this endpoint now, I had a bug with one of my PreviewProviders for my recommendations view that was not using my "mock" data, and instead was using an un-authenticated SpotifyAPI with my client ID. I've since fixed this issue, but this led to the recommendation endpoint to 429 across all sessions that used my client ID.

I'll do some more testing with handleEvents, I'm still learning Combine and using this library has been a great learning opportunity for me!

I'd still like to be able to disable the retry mechanism for specific endpoints (like this one), or an upper limit on retry-after. If you would be open to a merge request for that, I'd be happy to look into it.

[Peter-Schorn]

Fixed in latest commit on master:

Added maxRetryDelay parameter with default value of 3 minutes.

// SpotifyAPI
public init(
    authorizationManager: AuthorizationManager,
    maxRetryDelay: Int = 180 /* 3 minutes */,
    networkAdaptor: (
        (URLRequest) -> AnyPublisher<(data: Data, response: HTTPURLResponse), Error>
    )? = nil
)

[Peter-Schorn]

maxRetryDelay has been implemented in 3.0.1.