Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

player.pausePlayback(deviceId) throws SyntaxError: Unexpected token 'G', "RpqtSNBrob"... is not valid JSON #127

Open
ellingtonjp opened this issue Aug 26, 2024 · 5 comments

Comments

@ellingtonjp
Copy link

Issue
Calling player.pausePlayback(deviceId) leads to the following error:

SyntaxError: Unexpected token 'G', "GpqtUNZroN"... is not valid JSON
    at JSON.parse (<anonymous>)
    at DefaultResponseDeserializer.deserialize (/Users/jon/code/batters-up/server/node_modules/@spotify/web-api-ts-sdk/src/serialization/DefaultResponseDeserializer.ts:8:31)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at PlayerEndpoints.putRequest (/Users/jon/code/batters-up/server/node_modules/@spotify/web-api-ts-sdk/src/endpoints/EndpointsBase.ts:16:16)
    at PlayerEndpoints.pausePlayback (/Users/jon/code/batters-up/server/node_modules/@spotify/web-api-ts-sdk/src/endpoints/PlayerEndpoints.ts:58:9)
    at Object.<anonymous> (/Users/jon/code/batters-up/server/src/routes/spotify.ts:51:9)

Reproducing

  • start playing a song on any spotify device
  • call player.pausePlayback(deviceId)

Cause

This looks like an issue with the web API. This library expects a JSON response, but calling the pause endpoint returns non-JSON:

curl --location --request PUT 'https://api.spotify.com/v1/me/player/pause' \
--header 'Authorization: ••••••'
=> oPClBVDzidwlAlSqmFF-zKNO62E

The web API docs for /me/player/pause say it should return an empty response, but it's returning some sort of ID.

Solution

I guess one of the following should happen:

  1. update the web API /me/player/pause endpoint to return empty
  2. allow this library to handle non-JSON responses (and if /me/player/pause isn't changed at least the docs should be updated to say it returns non-empty)

I'm happy to submit a PR if the solution is 2, but it seems like 1 is the correct approach.

@y-lobau
Copy link

y-lobau commented Sep 11, 2024

Same for me. I checked the previous version, 1.1.2 - not working either.

@chriswnl
Copy link

chriswnl commented Sep 15, 2024

I'm getting this too. The Ruby version of the api was updated last week to account for this.

The web API docs for /me/player/pause say it should return an empty response, but it's returning some sort of ID.

It seems something changed on the api itself without docs being updated. The docs also say the response should be a 204 response but it's returning a 200.

@DopiGFX
Copy link

DopiGFX commented Sep 16, 2024

Scince this seems to be happening to nearly all Requests with empty Responses, I suggest implementing a solution to Catch this Error before it breaks other parts of the Code:

I created a Custom Deserializer by taking the DefaultResponseSerializer and extending it with a try-catch to prevent the SyntaxError thrown by JSON#parse. My class Looks like that:

class UnhealthyResponseDeserializer implements IResponseDeserializer {
  async deserialize<T>(response: Response): Promise<T> {
    const text = await response.text();
    if (text.length > 0) {
      try {
        const json = JSON.parse(text) as T;
        return json;
      } catch (e: any) { }
    }

    return null as T;
  }
}

As far as I know, this unhealthy Responses are only sent from API's with an "empty response" (like Pause Playback) so this Deserializer should cause no issue to any Requests by returning null when the JSON#parse fails. Healthy Requests are being deserialized the same way as before so no changes needed at any other code parts.

To use this, just give your SpotifyAPI an instance of the fixed Deserializer:

SpotifyApi.withAccessToken(clientId,  accessToken, { deserializer: new UnhealthyDeserializer() });

I already created a Pull Request to get this fixed in the DefaultResponseDeserializer itself: #128
Feel free to review it.

@chriswnl
Copy link

chriswnl commented Sep 17, 2024

I had tried it with a catch, @DopiGFX trying to return 'text' as {result: 'text'} but couldn't get that working so ended up with this in js.

export class MyDeserializer {
  async deserialize(e) {
    const t = await e.text();
    if (t.length > 0 && this.isParseable(t)) {
      const e = JSON.parse(t)
      return e
    }
    return null
  }

  isParseable(string){
    try {
      const e = JSON.parse(string)
      return true
    } catch (error) {
      return false
    }
  }
}

@FabioGNR
Copy link

We can check for the content type set in the response and if it's not json do not attempt to parse it:

export default class FixedResponseDeserializer
  implements IResponseDeserializer
{
  public async deserialize<TReturnType>(
    response: Response,
  ): Promise<TReturnType> {
    const text = await response.text();

    const contentType = response.headers.get("content-type") ?? '';

    if (text.length > 0 && contentType.includes("application/json")) {
      const json = JSON.parse(text);
      return json as TReturnType;
    }

    return null as TReturnType;
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants
@chriswnl @FabioGNR @ellingtonjp @y-lobau @DopiGFX and others