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

CancelFn isn't cancelling the stream with Expo 52 #1363

Closed
hollanderbart opened this issue Dec 5, 2024 · 9 comments
Closed

CancelFn isn't cancelling the stream with Expo 52 #1363

hollanderbart opened this issue Dec 5, 2024 · 9 comments

Comments

@hollanderbart
Copy link

hollanderbart commented Dec 5, 2024

Describe the bug

When I've created a createCallbackClient callback client, and I created a stream, a CancelFn will be returned. If I want to cancel this function, the stream isn't cancelled.

To Reproduce

  • Create a callback client
  • Setup a stream
  • Wait until the first message arrives
  • After the first message has arrived, try to cancel the stream
  • Stream is still continuing

Environment (please complete the following information):

Connectrpc/connect in combination with React Native

unsubscribeRef.current = deviceClientCallback.streamDevice(
...,
(messageResponse: StreamDeviceResponse) => {
   unsubscribeRef.current() //this will not cancel the stream
})
unsubscribeRef.current() //this will cancel the stream, only before the first message has arrived. 
@hollanderbart hollanderbart added the bug Something isn't working label Dec 5, 2024
@hollanderbart
Copy link
Author

hollanderbart commented Dec 5, 2024

@timostamm Can you please help me here? Maybe I'm just doing something wrong here. But it seems that after receiving a message, this will make the abort / cancel invalid.

@srikrsna-buf
Copy link
Member

Hey @hollanderbart Can you share a reproducible example? We do have a test that checks for this case.

@hollanderbart
Copy link
Author

hollanderbart commented Dec 5, 2024

Not yet. I can create one tomorrow. Is there a public stream call I can use?

I think it's a race condition because after the first message has arrived, the cancel isn't working.
I'm sure the test will do something, but does it check if the stream is cancelled on the network level?

@srikrsna-buf
Copy link
Member

srikrsna-buf commented Dec 5, 2024

Is there a public stream call I can use?

Yes, you use this rpc hosted at https://demo.connectrpc.com

I'm sure the test will do something, but does it check if the stream is cancelled on the network level?

It indirectly checks this by validating the responses it is supposed to receive. We also pass the signal to fetch.

@hollanderbart
Copy link
Author

Repo: https://github.com/hollanderbart/BusinessCards-Connect

The logs that it print is:

 (NOBRIDGE) LOG  {"$typeName": "connectrpc.eliza.v1.IntroduceResponse", "sentence": "Hi John Doe. I'm Eliza."}
 (NOBRIDGE) LOG  Eliza introduce stream connected
 (NOBRIDGE) LOG  Trying to cancel the stream
 (NOBRIDGE) LOG  {"$typeName": "connectrpc.eliza.v1.IntroduceResponse", "sentence": "Before we begin, John Doe, let me tell you something about myself."}
 (NOBRIDGE) LOG  Eliza introduce stream connected
 (NOBRIDGE) LOG  Trying to cancel the stream
 (NOBRIDGE) LOG  {"$typeName": "connectrpc.eliza.v1.IntroduceResponse", "sentence": "I was one of the first programs capable of attempting the Turing test."}
 (NOBRIDGE) LOG  Eliza introduce stream connected
 (NOBRIDGE) LOG  Trying to cancel the stream
 (NOBRIDGE) LOG  {"$typeName": "connectrpc.eliza.v1.IntroduceResponse", "sentence": "How are you feeling today?"}
 (NOBRIDGE) LOG  Eliza introduce stream connected
 (NOBRIDGE) LOG  Trying to cancel the stream
 (NOBRIDGE) LOG  [FastEncoder] (assureJSILoaded) JSI install: Installed
 (NOBRIDGE) ERROR  undefined

@hollanderbart
Copy link
Author

Or is the need for a custom Transport necessary? Can the React Native example app please updated using the latest expo/fetch implementation? @timostamm Thanks!

https://github.com/connectrpc/examples-es/blob/cd9a4d1b3bc069b13a907880511b7cbb49cfb965/react-native/app/custom-transport.ts

@timostamm
Copy link
Member

Hey Bart, for a very long time, React Native didn't implement fetch, only XMLHttpRequest. That's why we added the custom transport as an example. It implements gRPC-Web unary calls via XMLHttpRequest.

I'm very happy that RN finally has fetch. This means it will be possible to use server-streaming RPCs via the Connect or gRPC-Web protocols, in addition to unary RPCs. Updating the example is tracked in connectrpc/examples-es#2134.

If the CancelFn returned by the callback client doesn't abort the request in RN, this is a bug in the fetch implementation of RN / expo, since we continually run tests asserting that it does, on various web browsers and Node.js.

Can you try to reproduce the problem with a minimal example that uses fetch directly, and file the issue against expo?

@timostamm timostamm changed the title CancelFn isn't cancelling the stream CancelFn isn't cancelling the stream with Expo 52 Dec 9, 2024
@timostamm timostamm removed the bug Something isn't working label Dec 9, 2024
@hollanderbart
Copy link
Author

hollanderbart commented Dec 9, 2024

I've changed my repo to implement a simple httpbin stream.

Also I've created an issue report on Expo here

@chrispine
Copy link
Contributor

I'm going to go ahead and close this, but feel free to reopen if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants