Fix uploadData calling fetchCredentials when token has not expired

[Omnicpie]

Currently, as part of the flow to build a request for uploadData, we call getCredentials() within AbstractRemote. This getCredentials function does not check / use the currently stored set of credentials unless there is a time in which the developer provides expiresAt as part of their fetchCredentials() response, Since this is not included in the typing, its relatively unlikely that this would be the case, so for most users we will be making excess calls to fetchCredentials when the current set of stored creds is still valid.

This PR expands the getCredentials() request to get the current token stored, decode the JWT data to get the current exp field (if available), and perform the same check that would happen for a manually provided expiresAt field.

Testing

I tested this using the example-vite demo with some minor alterations, along with a self-hosted powersync instance.

Below is a code snippet of the main changes to the example, mostly just giving a valid token and endpoint to the return of fetchCredentials. There was also an addition of a button to trigger an insert.

Before changes

Before the change showed the issue well, when we called uploadData (via pressing the button to insert an action), we could see that it would log insert for the uploadData method, but also that it would log fetch for the fetchCredentials method:

After changes

Post changes, when calling the endpoint with a valid token, we could see that insert was still logged, but the fetch log was no longer present, due to exp on the token still being valid.

[changeset-bot]

⚠️ No Changeset found

Latest commit: 9a5b18a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[Chriztiaan]

Hey @Omnicpie, I think that while this PR does work - it's possible that we might want to approach it slightly differently.
If consider what we do in our Dart SDK (example 1), we interrogate the status when setting up the stream and invalidate the credentials on a 401. It looks like we do this check/invalidate in a few places (streaming_sync.dart and connector.dart).
This results in the same behaviour as what you have here.

You are welcome to update this PR to match the Dart implemention, but it might be a bit of work though. Otherwise we do have this work in our backlog and will get to this in the near future.

[Omnicpie]

hey @Chriztiaan, if I'm understanding those dart implementations correctly, does that mean that we always assume that credentials are OK if they are present (in dart land), and only in the case that we have 401'd and wiped the creds, then we generate new ones on the next request?

if that is the case, what 401 would that be? Is it only the 401 from the sync service itself, or would an error from the uploadData function also trigger that functionality?

[Chriztiaan]

hey @Chriztiaan, if I'm understanding those dart implementations correctly, does that mean that we always assume that credentials are OK if they are present (in dart land), and only in the case that we have 401'd and wiped the creds, then we generate new ones on the next request?

Yes, we depend on the 401 to determine whether the credentials are invalid or not.

if that is the case, what 401 would that be? Is it only the 401 from the sync service itself, or would an error from the uploadData function also trigger that functionality?

From our side we are concerned about the powersync stream 401s. Users would normally manage their own session invalidation for uploads - but it could be the same, just that we don't enforce it. One possible challenge is detecting this status code for websockets. I haven't looked into this as of yet.