close reason should be optional and options.shouldRetry -> bool as a replacement for isFatalConnectionProblem - #418
Conversation
…n with a code without providing any reason - adding option shouldRetry -> bool that is meant to replace isFatalConnectionProblem(deprecated in the original javascript library) . this way we can configure to override and reconnect on certain fatal errors
|
Sometimes we make updates to our server and we need to reset the program. On closing server sends close code 1002 and no reason. PS. I know this might be confusing, but this PR is for the new websocket protocol: |
|
@juancastillo0 what do you think? |
|
Hi! Looks great! The implementation is a little different form the reference, but the differences make sense. There are no additional tests, however the changes are kind of incremental. Not sure if we should treat it as a breaking change, since the close reason was non-nullable. We could also make it non-null and pass an empty String (or the close code as a String) when the server does not return a reason, but I personally don't like it. And I suppose it would still be a breaking change since the If we make a breaking change maybe we could accommodate other changes like this one #411 (comment) and I am not sure what is the status of this PR #412. In general I think it could be merge, not sure what you think about the test and the breaking change. |
|
@catapop84 please pull the latest changes from master to CI succeeds |
chore: update lints for dart 3.1 (gql-dart#421)
|
@knaeckeKami done |
|
@knaeckeKami can you try one more time. Should be valid this time. |
This PR fixes 2 problems
reason: socket.closeReason!is expected to exists.isFatalConnectionProblemBased on their documentation: https://github.com/enisdenjo/graphql-ws/blob/master/src/client.ts , shouldRetry can override fatal close code which is the default implementation.