Skip to content

Commit c6124a6

Browse files
committed
fix infinite loops where put/delete methods return no content
1 parent a3ecc12 commit c6124a6

File tree

4 files changed

+19
-15
lines changed

4 files changed

+19
-15
lines changed

src/commonMain/kotlin/com.adamratzman.spotify/http/Endpoints.kt

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,19 +84,19 @@ public abstract class SpotifyEndpoint(public val api: GenericSpotifyApi) {
8484
}
8585

8686
internal open suspend fun post(url: String, body: String? = null, contentType: String? = null): String {
87-
return execute(url, body, HttpRequestMethod.POST, contentType = contentType)
87+
return execute(url, body, HttpRequestMethod.POST, contentType = contentType, retryOnNull = false)
8888
}
8989

9090
internal open suspend fun put(url: String, body: String? = null, contentType: String? = null): String {
91-
return execute(url, body, HttpRequestMethod.PUT, contentType = contentType)
91+
return execute(url, body, HttpRequestMethod.PUT, contentType = contentType, retryOnNull = false)
9292
}
9393

9494
internal suspend fun delete(
9595
url: String,
9696
body: String? = null,
9797
contentType: String? = null
9898
): String {
99-
return execute(url, body, HttpRequestMethod.DELETE, contentType = contentType)
99+
return execute(url, body, HttpRequestMethod.DELETE, contentType = contentType, retryOnNull = false)
100100
}
101101

102102
@Suppress("UNCHECKED_CAST")
@@ -134,7 +134,7 @@ public abstract class SpotifyEndpoint(public val api: GenericSpotifyApi) {
134134

135135
handleResponse(document, cacheState, spotifyRequest, retry202) ?: run {
136136
if (retryOnNull) {
137-
execute<ReturnType>(url, body, method, false, contentType)
137+
execute<ReturnType>(url, body, method, false, contentType, retryOnNull)
138138
} else {
139139
null
140140
}
@@ -149,7 +149,8 @@ public abstract class SpotifyEndpoint(public val api: GenericSpotifyApi) {
149149
method,
150150
retry202,
151151
contentType,
152-
true
152+
true,
153+
retryOnNull
153154
)
154155
} else throw e
155156
}

src/commonMain/kotlin/com.adamratzman.spotify/http/HttpRequest.kt

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ public class HttpRequest constructor(
105105
val respCode = response.status.value
106106

107107
if (respCode in 500..599 && (retryIfInternalServerErrorLeft == null || retryIfInternalServerErrorLeft == -1 || retryIfInternalServerErrorLeft > 0)) {
108+
if (api?.spotifyApiOptions?.enableDebugMode == true) Console.debug("Received internal server error ${respCode}, attempting to retry (${retryIfInternalServerErrorLeft} tries left)")
108109
return@let execute(
109110
additionalHeaders,
110111
retryIfInternalServerErrorLeft =
@@ -115,6 +116,7 @@ public class HttpRequest constructor(
115116
// otherwise, if it's 5xx and retryIfInternalServerErrorLeft == 0 we just continue and fail
116117

117118
if (respCode == 429) {
119+
if (api?.spotifyApiOptions?.enableDebugMode == true) Console.debug("Received 429, attempting to retry")
118120
val ratelimit = response.headers["Retry-After"]!!.toLong() + 1L
119121
if (api?.spotifyApiOptions?.retryWhenRateLimited == true) {
120122
delay(ratelimit * 1000)
@@ -140,7 +142,7 @@ public class HttpRequest constructor(
140142
)
141143
}
142144

143-
val response = HttpResponse(
145+
val httpResponseToReturn = HttpResponse(
144146
responseCode = respCode,
145147
body = body,
146148
headers = response.headers.entries().map { (key, value) ->
@@ -153,11 +155,11 @@ public class HttpRequest constructor(
153155

154156
api?.spotifyApiOptions?.httpResponseSubscriber?.let { subscriber ->
155157
launch(currentCoroutineContext()) {
156-
subscriber(this, response)
158+
subscriber(this, httpResponseToReturn)
157159
}
158160
}
159161

160-
return response
162+
return httpResponseToReturn
161163
}
162164
} catch (e: CancellationException) {
163165
throw e

src/commonTest/kotlin/com.adamratzman/spotify/priv/ClientPlayerApiTest.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class ClientPlayerApiTest : AbstractTest<SpotifyClientApi>() {
8686

8787
val device = api.player.getDevices().first()
8888

89-
val trackId = "7lPN2DXiMsVn7XUKtOW1CS"
89+
val trackId = "1VsVY1ySdH3nVSWnLT5vCf"
9090
api.player.startPlayback(
9191
playableUrisToPlay = listOf(PlayableUri("spotify:track:$trackId")),
9292
deviceId = device.id
@@ -117,7 +117,7 @@ class ClientPlayerApiTest : AbstractTest<SpotifyClientApi>() {
117117
val device = api.player.getDevices().first()
118118
val playlist = api.playlists.getPlaylist("098OivbzwUNzzDShgF6U4A")!!
119119
api.player.startPlayback(playlistId = playlist.id) // two tracks
120-
val trackId = "7lPN2DXiMsVn7XUKtOW1CS"
120+
val trackId = "1VsVY1ySdH3nVSWnLT5vCf"
121121
api.player.addItemToEndOfQueue(trackId.toTrackUri(), device.id)
122122
delay(1000)
123123
api.player.skipForward() // skip first
@@ -139,7 +139,7 @@ class ClientPlayerApiTest : AbstractTest<SpotifyClientApi>() {
139139

140140
val device = api.player.getDevices().first()
141141

142-
val trackId = "7lPN2DXiMsVn7XUKtOW1CS"
142+
val trackId = "1VsVY1ySdH3nVSWnLT5vCf"
143143
val track = api.tracks.getTrack(trackId)!!
144144
api.player.startPlayback(
145145
playableUrisToPlay = listOf(PlayableUri("spotify:track:$trackId")),
@@ -226,7 +226,7 @@ class ClientPlayerApiTest : AbstractTest<SpotifyClientApi>() {
226226
api.player.startPlayback(
227227
playableUrisToPlay = trackUris
228228
)
229-
delay(1000)
229+
delay(2500)
230230
assertEquals(trackUris.first().id, api.player.getCurrentlyPlaying()?.item?.id)
231231
api.player.skipForward()
232232
delay(1000)
@@ -301,7 +301,7 @@ class ClientPlayerApiTest : AbstractTest<SpotifyClientApi>() {
301301
val toDevice = devices[1]
302302

303303
api.player.startPlayback(
304-
playableUrisToPlay = listOf(PlayableUri("spotify:track:7lPN2DXiMsVn7XUKtOW1CS")),
304+
playableUrisToPlay = listOf(PlayableUri("spotify:track:1VsVY1ySdH3nVSWnLT5vCf")),
305305
deviceId = fromDevice.id
306306
)
307307
delay(1000)

src/jvmTest/kotlin/com/adamratzman/spotify/CommonImpl.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ actual suspend fun buildSpotifyApi(): GenericSpotifyApi? {
1919

2020
val optionsCreator: (SpotifyApiOptions.() -> Unit) = {
2121
this.enableDebugMode = logHttp
22+
this.enableLogger = true
2223
this.httpResponseSubscriber = { request, response ->
23-
println("=== request ${requestNumber} ===")
24+
/*println("=== request ${requestNumber} ===")
2425
println(request)
25-
println(response)
26+
println(response)*/
2627
requestNumber++
2728
}
2829
}

0 commit comments

Comments
 (0)