Address null check error and prevent duplicate events on socket reconnect

zino-hofmann / graphql-flutter

A GraphQL client for Flutter, bringing all the features from a modern GraphQL client to one easy to use package.

https://zino-hofmann.github.io/graphql-flutter

MIT License

3.23k stars 608 forks source link

Address null check error and prevent duplicate events on socket reconnect #1410

Closed aarsham closed 3 months ago

aarsham commented 4 months ago

Fixes

Removed unnecessary null checks(!) on SocketChannel instance in SocketClient onConnectionLost() method which cause an error when client cannot connect (#1379). this prevents reconnecting when there is a working connection later.
Resolved an issue in WebSocketClient where multiple listeners were added to waitForConnectedState when socket reconnects after subscription been requtesed with disconnected socket, causing duplicate events to be received by subscriptions (#989) (thanks to @stardarLeung).
Fixed an issue in fetchMore where results were not added to the stream when fetchMoreOptions.updateQuery returned null.

vincenzopalazzo commented 3 months ago

please can fix the Ci ? :) I think it is missing a simple make fmt

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 64.61%. Comparing base (eebac38) to head (2891d5b).

:exclamation: Current head 2891d5b differs from pull request most recent head acdaabc. Consider uploading reports for the commit acdaabc to get more accurate results

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1410 +/- ## ========================================== - Coverage 64.83% 64.61% -0.23% ========================================== Files 41 41 Lines 1732 1718 -14 ========================================== - Hits 1123 1110 -13 + Misses 609 608 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

aarsham commented 3 months ago

Some tests are failing because of this PR (#1394), which did not have all necessary checks run for it.

vincenzopalazzo commented 3 months ago

@aarsham Waiting for https://github.com/zino-hofmann/graphql-flutter/pull/1419

aarsham commented 3 months ago

@vincenzopalazzo can you merge this please?

vincenzopalazzo commented 3 months ago

can you merge this please?

Rebase the PR on top of the current master in this way we can verify that the Ci is happy, thanks