spotify / spotify-web-api-ts-sdk

A Typescript SDK for the Spotify Web API with types for returned data.
Other
325 stars 59 forks source link

Expired Token Issue (Not Auto Refreshing) #64

Open rishabhbizzle opened 10 months ago

rishabhbizzle commented 10 months ago

Hey, I have been using this SDK for a week, and now this error has come up. I know the token is expired but you mentioned that the SDK does auto-refresh the token so why is it not doing that?

image

image
rishabhbizzle commented 10 months ago

can someone help me with this issue ??

davidwhitney commented 10 months ago

Interesting, it looks like the ClientCredentialsStrategy isn't providing a refresh mechanism - this is because the Bearer token returned from a client credentials auth flow doesn't include a refresh token.

I'm imagining that your token is expiring after 60 minutes of activity. Is that what you're seeing?

Luckily, while it's not strictly a "token refresh", because client creds flow has both the client Id and secret it's easy to just request another token on expiry so should be a small change to that strategy - I've got a WIP PR ready to go.

rishabhbizzle commented 10 months ago

Interesting, it looks like the ClientCredentialsStrategy isn't providing a refresh mechanism - this is because the Bearer token returned from a client credentials auth flow doesn't include a refresh token.

I'm imagining that your token is expiring after 60 minutes of activity. Is that what you're seeing?

Luckily, while it's not strictly a "token refresh", because client creds flow has both the client Id and secret it's easy to just request another token on expiry so should be a small change to that strategy - I've got a WIP PR ready to go.

It's been 2 days and I'm still getting this error :(

davidwhitney commented 10 months ago

Have you restarted the process that has the cached token during those two days?

davidwhitney commented 10 months ago

^ this should now be fixed on the next package release.

steezplusplus commented 10 months ago

Screen Shot 2023-09-05 at 6 14 41 PM

Same code as the OP uses

davidwhitney commented 10 months ago

@steezplusplus - the PR hasn't been published to NPM yet, so I'd expect no difference yet!

Just to confirm though - this is a long running process (process running for over 1 hour) then failures start, but restarting the process makes it work again for one hour?

Unfortunately there's not much context provided the above so I'm guessing!

rishabhbizzle commented 10 months ago

@steezplusplus - the PR hasn't been published to NPM yet, so I'd expect no difference yet!

Just to confirm though - this is a long running process (process running for over 1 hour) then failures start, but restarting the process makes it work again for one hour?

Unfortunately there's not much context provided the above so I'm guessing!

After restarting the process it's still giving the same error

davidwhitney commented 10 months ago

@rishabhbizzle and is this running inside a browser context? I'm guessing from your stack trace.

I think that's perhaps consistent - it's loading the outdated token from the cache, doesn't have a refresh mechanism, so it's trying to use it and erroring. If you clear your local storage, does it work again?

I've just started running a test now that'll run for a few hours to try verify...

rishabhbizzle commented 10 months ago

@rishabhbizzle and is this running inside a browser context? I'm guessing from your stack trace.

I think that's perhaps consistent - it's loading the outdated token from the cache, doesn't have a refresh mechanism, so it's trying to use it and erroring. If you clear your local storage, does it work again?

I've just started running a test now that'll run for a few hours to try verify...

I tried after clearing local storage it still doesn't work. I am using this in my Next.js project this is how the code structure looks like

image image
davidwhitney commented 10 months ago

How bizarre. The Client Credentials strategy always returns a new one using your client key and client secret when the process restarts and the cache is empty. I'm presuming you've had this working at least once and it's not just a bad error message?

rishabhbizzle commented 10 months ago

How bizarre. The Client Credentials strategy always returns a new one using your client key and client secret when the process restarts and the cache is empty. I'm presuming you've had this working at least once and it's not just a bad error message?

Yeah it was working perfectly fine earlier for 2-3 days using this same code structure but now suddenly it doesn't work. The error is the same as I mentioned earlier. Btw is there something wrong with this structure and my implementation??

davidwhitney commented 10 months ago

No, your implementation looks totally fine at a glance. It's weird that it'd work for a couple of days - I wonder if your API key has changed / been revoked / been modified in some way? (I don't work for Spotify; I can't help you there!)

All I could think of is bad cached data, but if you've cleared local storage, and still have the keys and it doesn't work fresh, I'm at a bit of a loss - it just suggests the API keys themselves are bad...

davidwhitney commented 10 months ago

Quick update, the test run I did with some additional refresh code ran for 10 hours happily - have you verified your token info is still correct?

oogunjob commented 10 months ago

I'm also experiencing the same issue, even when using client credentials auth method. I can run, sdk.authorize(), but when trying to run sdk.currentUser.profile(), I get this error message: Error: Bad or expired token. This can happen if the user revoked a token or the access token has expired. You should re-authenticate the user.

rishabhbizzle commented 10 months ago

Quick update, the test run I did with some additional refresh code ran for 10 hours happily - have you verified your token info is still correct?

Yes I verified even after manually refreshing token using authenticate() function the new token gets issues but it still doesn't work and continues to give the same "Bad or Expired Token"

wintercounter commented 9 months ago

Has anyone found a solution to this? Now I have to restart my server with CRON once a day to overcome this which is less than ideal. Not to mention I'm losing my in-memory cache with that.

wintercounter commented 9 months ago

You guys are trying to use it with Next.js 13, right? I believe I know what's the problem. Next caches all fetch requests by default. cache: 'force-cache' is the default value for all fetch requests. We cannot control fetch options for Auth Strategies, so there is no way to solve this in userland.

I tried adding cache: 'no-store' inside the package's dist folder, cleared webpack and swc cache and it seems like it solves this issue: https://github.com/spotify/spotify-web-api-ts-sdk/blob/main/src/auth/ClientCredentialsStrategy.ts#L67C31-L67C31

It's a pretty dumb default if you ask me (like many other things in Next). Not sure adding cache: no-store to the API client is the right solution as it's a non-standard value. Probably using the user-provided fetch for auth requests also would be a better way.

wintercounter commented 9 months ago

I just want to give an update on this. I added a script to my codebase to monkey-patch the source files, and since then I don't have token issues anymore, the app has been running on prod for 4 days now. Next should not cache POST requests or when the Authorization header is present. It doesn't create a cache file, but still, something is wrong under the hood there. I'm thinking of creating an issue for them, but the repro is not an easy case.

Here's the script if anyone else wants to experiment with it:

import fs from 'node:fs/promises'

const spotifyApiDistPath = './node_modules/@spotify/web-api-ts-sdk/dist'
const spotifyApiMjsPath = `${spotifyApiDistPath}/mjs/auth/ClientCredentialsStrategy.js`
const spotifyApiCjsPath = `${spotifyApiDistPath}/cjs/auth/ClientCredentialsStrategy.js`
const paths = [spotifyApiMjsPath, spotifyApiCjsPath]

const patch = async path => {
    const content = await fs.readFile(path, 'utf-8')
    await fs.writeFile(path, content.replace(/body: bodyAsString\s/g, `body: bodyAsString, cache: 'no-store'`))
}

paths.forEach(patch)

console.log('Patched Spotify API')
jackblanc commented 6 months ago

Can confirm the above solution worked for me. I added this script as a "postinstall" command in my package.json.

rishabhbizzle commented 4 months ago

I just want to give an update on this. I added a script to my codebase to monkey-patch the source files, and since then I don't have token issues anymore, the app has been running on prod for 4 days now. Next should not cache POST requests or when the Authorization header is present. It doesn't create a cache file, but still, something is wrong under the hood there. I'm thinking of creating an issue for them, but the repro is not an easy case.

Here's the script if anyone else wants to experiment with it:

import fs from 'node:fs/promises'

const spotifyApiDistPath = './node_modules/@spotify/web-api-ts-sdk/dist'
const spotifyApiMjsPath = `${spotifyApiDistPath}/mjs/auth/ClientCredentialsStrategy.js`
const spotifyApiCjsPath = `${spotifyApiDistPath}/cjs/auth/ClientCredentialsStrategy.js`
const paths = [spotifyApiMjsPath, spotifyApiCjsPath]

const patch = async path => {
    const content = await fs.readFile(path, 'utf-8')
    await fs.writeFile(path, content.replace(/body: bodyAsString\s/g, `body: bodyAsString, cache: 'no-store'`))
}

paths.forEach(patch)

console.log('Patched Spotify API')

How did you manage to make this work in production? assuming you used vercel for the deployment @wintercounter @jackblanc

image

Like I've added the patch script before next build in build command but it still giving this error on prod

image
wintercounter commented 4 months ago

I'm running it after install.

image

https://github.com/wintercounter/dnbop

rishabhbizzle commented 4 months ago

I'm running it after install.

image

https://github.com/wintercounter/dnbop

tried this and it's still not working :(

wintercounter commented 4 months ago

Seems like to me you get this immediately, so it doesn't even wait for the token to get expired. So maybe it's a different issue?

It's also possible that this doesn't work with the latest version of the client. Maybe the code was changed recently and it fails to replace. Just a guess. Maybe you can try with the version I'm using.

Does the script replacing it for you locally?

rishabhbizzle commented 4 months ago

Yes I ran the script locally 12 days ago (with the latest version of the client) and it fixed the issue and working smoothly from then on local

strenkml commented 3 months ago

I am having this error as well. Except instead of working for a short period of time, mine never works.

I am authenticating with this: SpotifyApi.withClientCredentials(this.clientId, this.clientSecret);

I created the app in the dev panel and copied the clientId and clientSecret to my code. I have confirmed that the method is getting passed the correct values.

I should note that I am using plain node.js, not Next.js