Fix connect() resolving early with Rust client

[simolus3]

With the JavaScript sync client, the promise returned by connect() is supposed to return after a connection has been established and the first sync line received (this is different to all our other SDKs, which return as soon as a connection task has been set up and started).

The way this behavior is implemented is that connect() registers a statusUpdated handler that completes once the connected status is changed. This works with the JavaScript client because it only changes fields that actually need to change. With Rust however, we receive a full SyncStatus snapshot instead (so we call updateSyncStatus with a complete status that will always have connected set - even to false initially).

The workaround feels a bit hacky, but since something like statusUpdated doesn't really work with the Rust client at all, the best approach I could come up with is to exclude connected from the options passed to updateSyncStatus in the initial connection setup status.

This fixes a bug reported on Discord. However, I could not report longer connection times with the new Rust client after adding the connect() call with the fix.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5c57ea8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@powersync/common	Patch
@powersync/node	Patch
@powersync/op-sqlite	Patch
@powersync/react-native	Patch
@powersync/tanstack-react-query	Patch
@powersync/web	Patch
@powersync/diagnostics-app	Patch

Not sure what this means? Click here to learn what changesets are.

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

[rkistner]

I did some testing to see the exact status sequence during the initial connection attempt on the various versions. For these tests, I added an artificial delay of 1500ms before sending the first checkpoint message on the service (after connecting). The first number in the logs here indicate time since calling connect(). Testing using a modified react-supabase-todolist demo that doesn't actually connect to supabase.

main branch, JS implementation:

0 'Calling connect()'
167 'Status' {connecting: false, connected: false, downloading: false, uploading: false}
[PowerSyncDatabase] Attempting to connect to PowerSync instance
229 'Status' {connecting: true, connected: false, downloading: false, uploading: false}
[PowerSyncStream] requesting lock for  streaming-sync-sync-example.db
[PowerSyncStream] Streaming sync iteration started
[PowerSyncStream] Requesting stream from server
[PowerSyncStream] calling the last port client provider for credentials
[PowerSyncStream] Stream established. Processing events
1898 'Status' {connecting: false, connected: true, downloading: false, uploading: false}
1901 'connect() resolved. connected?' true
1902 'Status' {connecting: false, connected: true, downloading: false, uploading: true}
1902 'Status' {connecting: false, connected: true, downloading: true, uploading: true}
[PowerSyncStream] validateChecksums priority, checkpoint, result item undefined {last_op_id: '107100665', buckets: Array(3)} {result: '{"valid":true,"failed_buckets":[]}'}
1904 'Status' {connecting: false, connected: true, downloading: true, uploading: false}
[PowerSyncStream] validated checkpoint {last_op_id: '107100665', buckets: Array(3)}
1915 'Status' {connecting: false, connected: true, downloading: false, uploading: false}
2899 'Status' {connecting: false, connected: true, downloading: false, uploading: true}
2901 'Status' {connecting: false, connected: true, downloading: false, uploading: false}

Comments:

1. First status update after calling connect() has connecting: false, which feels wrong.
2. It feels weird to only have connected: true once the first line was received, rather than when the connection was established.
3. We only get downloading: true a short while after the first checkpoint line was received. I feel it would be better to have downloading: true together with the first connected: true status, since we're effectively checking for new changes at that point.
4. Around a second after the connection is established, there is a spike of uploading: false -> true -> false, even though there are no changes to upload. This is consistent for me.

main branch, RUST implementation:

0 'Calling connect()'
185 'Status' {connecting: false, connected: false, downloading: false, uploading: false}
[PowerSyncDatabase] Attempting to connect to PowerSync instance
268 'Status' {connecting: true, connected: false, downloading: false, uploading: false}
[PowerSyncStream] requesting lock for  streaming-sync-sync-example.db
[PowerSyncStream] Initial connect attempt did not successfully connect to server
l[PowerSyncStream] calling the last port client provider for credentials
390 'connect() resolved. connected?' false
1900 'Status' {connecting: false, connected: true, downloading: true, uploading: false}
[PowerSyncStream] Validated and applied checkpoint
1908 'Status' {connecting: false, connected: true, downloading: false, uploading: false}

Comments:

1. This shows the core issue fixed by this PR: connect() resolving before we have connected: true.
2. This does have connected: true, downloading: true as the first status update after connecting, which is nice.
3. No uploading status being toggled after connecting.

this branch, RUST implementation:

0 'Calling connect()'
171 'Status' {connecting: false, connected: false, downloading: false, uploading: false}
[PowerSyncDatabase] Attempting to connect to PowerSync instance
244 'Status' {connecting: true, connected: false, downloading: false, uploading: false}
[PowerSyncStream] requesting lock for  streaming-sync-sync-example.db
[PowerSyncStream] calling the last port client provider for credentials
1883 'Status' {connecting: false, connected: true, downloading: true, uploading: false}
1887 'connect() resolved. connected?' true
1887 'Status' {connecting: false, connected: true, downloading: true, uploading: true}
[PowerSyncStream] Validated and applied checkpoint
1890 'Status' {connecting: false, connected: true, downloading: false, uploading: true}
1892 'Status' {connecting: false, connected: true, downloading: false, uploading: false}
2883 'Status' {connecting: false, connected: true, downloading: false, uploading: true}
2886 'Status' {connecting: false, connected: true, downloading: false, uploading: false}

Comments:

1. This shows the issue is fixed.
2. This does also have connected: true, downloading: true as the first status update after connecting.
3. Uploading status is changed to true just after the connection is established and again a second later. I don't know why this is different from the main branch, but I consistently get this behavior here and not on the main branch.

Overall comments

1. This fix is a clear improvement, so I'm happy for this to be merged and released as-is. With this change the rust implementation's status reporting appears to be on-par or better than the JS implementation.
2. I also don't see any actual performance difference here in connection times between the rust and JS implementations.

However, I think we should improve the status reporting further:

1. The first status update should have connecting: true.
2. We should have connected: true and resolve the connect() Promise as soon as the connection is established (or perhaps the rsocket stream is opened?), not only when we receive the first sync line, as there can be a significant delay before we receive the checkpoint in some cases.
3. Together with the above, we should have downloading: true for the first status update where we have connected: true (already the case with the rust implementation, but not the JS one).
4. We should not have uploading: true when there isn't data to upload.
5. There appears to be a significant delay (over 200ms) before getting connecting: true on all versions here. I'm not sure where that delay comes from - some of it may be overhead from the demo app or using a debug build.

[simolus3]

The first status update should have connecting: true.

At least in the node test (connect() waits for connection), that appears to be the case.

We should have connected: true and resolve the connect() Promise as soon as the connection is established (or perhaps the rsocket stream is opened?)

Fixed for Rust now.

Together with the above, we should have downloading: true for the first status update where we have connected: true

I don't agree with this one in combination with the one above. How are we downloading rows before the first checkpoint line? The Rust client sets downloading: true from the checkpoint until a checkpoint_complete message which sounds like the correct behavior to me. Which behavior would you expect, downloading: true initially until the first checkpoint is completed and then again for subsequent ones?

We should not have uploading: true when there isn't data to upload.

Fixed!

[stevensJourney]

From my side this looks good to me.

[rkistner]

I don't agree with this one in combination with the one above. How are we downloading rows before the first checkpoint line? The Rust client sets downloading: true from the checkpoint until a checkpoint_complete message which sounds like the correct behavior to me. Which behavior would you expect, downloading: true initially until the first checkpoint is completed and then again for subsequent ones?

For this one, the general issue is that if you get {connecting: false, connected: true, downloading: false}, there is no nice way to know if it is "we are expecting a checkpoint soon which may contain data to download" (which is the case on initial connection) versus "connection is idle" (already received a checkpoint). This could potentially be solved by adding another status field. But either way this is not a regression, so happy to revisit later.