graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

StrawberryShake subscription returns before ready - timing issue

Open benbierens opened this issue 3 years ago • 4 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

HT_subscription

Context: Topic: Automated API/integration tests. Project: dotgraphee Issue: StrawberryShake's subscription .Watch().Subscribe() seem to return before the system is ready to receive subscriptions. (Cleanup: .Subscribe() returns an IDisposable which is disposed by the test fixture TearDown. We didn't forget it. 😉 ) Workaround: Thread.Sleep(1000); after subscription is started. 😢

Steps to reproduce

  1. Set up a HotChocolate endpoint with subscriptions.
  2. Set up a StrawberryShake client to subscribe and trigger that subscription with a mutation.
  3. Create a test that calls the mutation immediately after starting the subscription, see image above. (dotgraphee is building a branch 'feature/strawberry-shake-client' that does step 1, 2 and 3.)
  4. Test may pass randomly or when run in isolation. Create several or run in loop to reproduce.

Relevant log output

No response

Additional Context?

No response

Product

Strawberry Shake

Version

12.6.2

benbierens avatar Mar 09 '22 06:03 benbierens

Do you have a small repro?

Watch essentiall returns an observable and subscribe registeres a delegate that receives updates from the entity store. You will get controll immediately. But may be I am misunderstanding what you expectation is in this case ...

If you use System.Reactive you can also convert the observable into an async stream and interate over it which will give you a blocking call until something arrives.

michaelstaib avatar Mar 09 '22 17:03 michaelstaib

I had flaky test similar to what you describe but maybe my setup was wrong. To do subscribe->mutation->wait for subscription response->assert kind of test I was doing something like this.

var timeout = TimeSpan.FromSeconds(5);
var cancellationTokenSource = new CancellationTokenSource(timeout);
var value = 0;
using var _ = Client.OnSaleBid.Watch(saleId).Subscribe(response =>
{
  value = response.Data.OnValue.Value;
  cancellationTokenSource.Cancel();
});

await Client.Foo.ExecuteAsync(CancellationToken.Token);

await Task.Delay(-1, cancellationTokenSource.Token).ContinueWith(_ => { }, CancellationToken.None);
value.Should().BePositive();

It usually happened in CI where multiple tests run in parallel.

kolpav avatar Apr 04 '22 06:04 kolpav

Delays are no good to deal with these kinds of tests.

michaelstaib avatar Apr 04 '22 07:04 michaelstaib

@michaelstaib Same error happened when I rewrote my test using System.Reactive I guess there is race condition somewhere or something in my fixture setup which is more likely because I had to copy paste code from HC to make strawberry shake to use test server's web socket client. When I was asking for help on slack you said "Wait for the subscription stitching pr" which I assume would make it easier to do. Is the PR already part of some release? I could then update my fixture to see if the error still happens.

kolpav avatar Apr 10 '22 04:04 kolpav