Supabase requests to non rest endpoints are added to the offline request queue and cause problems

[devj3ns]

When using the new Supabase integration, requests made by the Supabase client, which are unrelated to Brick are stored in the offline request queue. I would expect brick to only add requests to the offline request queue which are related to the remote database.

While testing my app offline, I noticed that POST requests to these endpoints were added to the offline queue:

https://xyz.supabase.co/auth/v1/token?grant_type=refresh_token https://xyz.supabase.co/auth/v1/logout?scope=local

[tshedor]

@devj3ns What is the risk/problem of auth requests being in the queue?

[devj3ns]

I believe that requests unrelated to Brick and not initiated by it should not be added to bricks offline request queue. Consequently, retrying these requests should not be handled by Brick. I have not tested it fully, but I can imagine that queuing auth request could lead to unexpected behavior in the future.

[tshedor]

@devj3ns In the internals of the SupabaseClient, there's potential for a separate auth client. However, this auth client will inherit the httpClient that's passed in, so it effectively can't be configured from the constructor.

The alternative to this would be maintaining a separate Supabase client for auth. Which is...honestly not that bad. I'm going to make a note of this in the documentation, but I don't know that it's a first-party feature. I'm very very reluctant to hardcode endpoints into the client that should not be retried. That's going to be brittle and will require long-term vigilance.

I'd also like to reach out to the Supabase team when this is done, and they may be willing to expose that authClient as an argument.

PR is #442

[devj3ns]

In the internals of the SupabaseClient, there's potential for a separate auth client ... they may be willing to expose that authClient as an argument

@tshedor What if they expose the restClient as an argument instead? Then we could pass the RestOfflineQueueClient and it would be ensured, that only requests to the rest endpoint would go through brick's offline queue client.

[devj3ns]

I found a bug which is related to this issue:

When trying to upload a file using the Supabase SDK:

Supabase.instance.client.storage.from('test').uploadBinary('file.txt', Uint8List.fromList('Hello World'.codeUnits));

the following exception is thrown:

Unhandled Exception: type 'MultipartRequest' is not a subtype of type 'Request' in type cast
E/flutter ( 5523): #0      RestOfflineQueueClient.send (package:brick_offline_first_with_rest/src/offline_queue/rest_offline_queue_client.dart:38:34)
E/flutter ( 5523): #1      AuthHttpClient.send (package:supabase/src/auth_http_client.dart:17:19)
E/flutter ( 5523): <asynchronous suspension>
E/flutter ( 5523): #2      RetryOptions.retry (package:retry/retry.dart:131:16)
E/flutter ( 5523): <asynchronous suspension>
E/flutter ( 5523): #3      Fetch._handleBinaryFileRequest (package:storage_client/src/fetch.dart:144:24)
E/flutter ( 5523): <asynchronous suspension>
E/flutter ( 5523): #4      StorageFileApi.uploadBinary (package:storage_client/src/storage_file_api.dart:91:22)

The reason is the following cast inside the RestOfflineQueueClient send method.

final cachePolicy = (request as http.Request).headers.remove(policyHeader);

There may be other issues when using Supabase functions or realtime, which currently also use bricks RestOfflineQueueClient. To me, this shows the importance of only using bricks RestOfflineQueueClient for requests to the REST endpoint.

[tshedor]

I'm not doubting the importance, but if you look at the source code there's only so much that we're capable of changing. Functions may also be cached, depending on the use of the response value.

[tshedor]

@devj3ns I'm not happy about it, but I've merged #442 with a fix that will, by default, ignore auth and storage paths. Functions are not ignored by default because they're a gray area. Merged, not published.

When this is at a release point, I'm going to raise the hack to the Supabase team so that they might expose independent clients for auth and functions and storage.

[devj3ns]

Thanks, @tshedor, I think that's the only way we can handle this issue currently. I just tested it and the issue is resolved.