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

New refresh tokens are not stored correctly when using cloud sync #693

Open
gcampax opened this issue Jul 15, 2021 · 4 comments
Open

New refresh tokens are not stored correctly when using cloud sync #693

gcampax opened this issue Jul 15, 2021 · 4 comments
Assignees
Labels
bug Something isn't working engine Issues with the core Genie engine P1 We're working on it right now

Comments

@gcampax
Copy link
Contributor

gcampax commented Jul 15, 2021

Not sure where the bug really is, so filing it here and then we'll find the right place.

Currently, tests for thingpedia-common-devices in Travis fail intermittently in Spotify, because API calls fail with "Refresh token revoked". A bit of investigation reveals that PKCE-enabled refresh tokens are single use only, and we need to store the new token when we get one. Something, somewhere, is failing to store the new token.

@gcampax gcampax added bug Something isn't working engine Issues with the core Genie engine labels Jul 15, 2021
@nrser nrser added the P2 We need to fix it (backlog) label Aug 3, 2021
@nrser
Copy link
Contributor

nrser commented Aug 10, 2021

@jmhw0123 think this might have something to do with the failure you were seeing on Travis?

@jmhw0123
Copy link
Contributor

@jmhw0123 think this might have something to do with the failure you were seeing on Travis?

Yeah. I think it's probably one of the reasons.

@nrser nrser added P1 We're working on it right now and removed P2 We need to fix it (backlog) labels Aug 10, 2021
@nrser
Copy link
Contributor

nrser commented Aug 10, 2021

Ok I bumped it up to P1, Spotify skill is one of our focuses and we're writing and running automated tests -- random failures undermine that.

@gcampax any idea what it may take to address this?

@gcampax
Copy link
Contributor Author

gcampax commented Aug 10, 2021

We need to investigate exactly where the sync goes wrong (could be in the cloud, could be in the client), and then fix that. The main issue is that the sync connection might drop before the client had a chance to upload the new refresh token to the cloud, in which case we never store the new refresh token. Next time we run the tests, we fetch the invalidated refresh token again and die. We might be able to force the scenario tests to sync with the cloud before it closes.

The other issue is that both unit tests and scenario tests run concurrently. If both refresh at the same time we'll get in trouble.

@gcampax gcampax self-assigned this Sep 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working engine Issues with the core Genie engine P1 We're working on it right now
Projects
None yet
Development

No branches or pull requests

3 participants