-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
feat(functions_client): Add SSE support to invoke method #905
Conversation
} else if (responseType == 'application/octet-stream') { | ||
data = await response.stream.toBytes(); | ||
} else if (responseType == 'text/event-stream') { | ||
data = response.stream; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here, I thought about adding .transform(const Utf8Decoder())
so that the developers don't have to add them by themselves, but Utf8Decoder()
has a allowMalformed
option, and I wanted to give the developers full control of what to do with the option, so I'm just returning raw ByteStream.
data = response.stream.transform(const Utf8Decoder());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation for data
in the FunctionResponse
should be updated as well.
request.headers[key] = value; | ||
}); | ||
if (bodyStr != null) request.body = bodyStr; | ||
final response = await (_httpClient?.send(request) ?? request.send()); | ||
final responseType = (response.headers['Content-Type'] ?? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the documentation it says, that all headers are converted to lowercase. So the first check should never succeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I deleted the response.headers['Content-Type'] ??
and ran the test, but it seems like the tests are failing when the mock server returns Content-Type
header in upper case, which suggests that the header keys are preserved the way it was sent from the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the documentation is correct, but we just skip the translation when mocking the response object directly. So just keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I accidentally approved)
@@ -1,4 +1,6 @@ | |||
library functions_client; | |||
|
|||
export 'package:http/http.dart' show ByteStream; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this, because I thought people might want to type cast the response.data
as ByteStream
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about adding the missing support for the stream in web to the doc as well? I already see an issue coming in here...
Thanks for your effort! But is there no possibility for it to work in web? I managed to make it work in web (with older gotrue/supabase-flutter version) using Maybe like it is done in here: pratikbaid3/flutter_client_sse#19 (comment) There is the option to provide an This would make it possible to support streaming in web too. |
@nietsmmar Actually, just by passing fetch client, I was able to stream SSE on the web! final client = FetchClient(mode: RequestMode.cors);
await Supabase.initialize(
url: supabaseUrl,
anonKey: supabaseKey,
debug: false,
httpClient: kIsWeb ? client : null,
); |
What kind of change does this PR introduce?
Adds support for streaming response of an edge function that returns Server Sent Events.
Fixes #894
What is the current behavior?
There is no support for SSE, and developers have to use other libraries to listen to SSE.
What is the new behavior?
Users can listen to SSE like the following:
Additional Context
On the web, you would need to pass a custom HTTP client like this: