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

Check for nullable keepAliveSubscription #772

Merged
merged 2 commits into from
Dec 26, 2020

Conversation

phuchuynhStrong
Copy link
Contributor

@phuchuynhStrong phuchuynhStrong commented Nov 17, 2020

Check for nullable keepAliveSubscription when disposing SocketClient.

Fixes

  • Fixed dispose function of SocketClient, it didn't check the nullable keepAliveSubscription before put it into Future.wait. If SocketClientConfig constructed with null inactivityTimeout, keepAliveSubscription won't be initialized which results in the exception in Future.wait.

@codecov
Copy link

codecov bot commented Nov 17, 2020

Codecov Report

Merging #772 (6adf61f) into beta (a3235a4) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta     #772      +/-   ##
==========================================
+ Coverage   50.48%   50.52%   +0.03%     
==========================================
  Files          34       34              
  Lines        1240     1241       +1     
==========================================
+ Hits          626      627       +1     
  Misses        614      614              
Flag Coverage Δ
graphql_client 54.36% <100.00%> (+0.04%) ⬆️
graphql_flutter 31.57% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...lib/src/links/websocket_link/websocket_client.dart 73.65% <100.00%> (+0.15%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a3235a4...6adf61f. Read the comment docs.

@micimize
Copy link
Collaborator

if this is an issue it would be better to apply it to all the items in the list, as they all appear to be nullable. Something like Future.wait([...].where((closing) => closing != null)).toList())

@phuchuynhStrong
Copy link
Contributor Author

Yup. It's better approach to handle this func. I updated the code.

@micimize micimize merged commit 4877b39 into zino-hofmann:beta Dec 26, 2020
@HofmannZ
Copy link
Member

🎉 This PR is included in version 4.0.0-beta.6 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@HofmannZ
Copy link
Member

🎉 This PR is included in version 4.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants