{"action":"created","comment":{"url":"https://api.github.com/repos/Workiva/w_transport/pulls/comments/65920032","id":65920032,"diff_hunk":"@@ -126,31 +103,58 @@ abstract class CommonWSocket extends Stream implements WSocket {\n   /// Sending additional data before this stream has completed may\n   /// result in a [StateError].\n   Future addStream(Stream stream) async {\n-    await sink.addStream(stream);\n+    return _outgoing.addStream(stream);\n   }\n \n   /// Closes the WebSocket connection. Optionally set [code] and [reason]\n   /// to send close information to the remote peer.\n   Future close([int code, String reason]) {\n-    if (_isClosed) return done;\n-    _isClosed = true;\n     shutDown(code: code, reason: reason);\n     return done;\n   }\n \n-  void closeSocket(int code, String reason);\n+  StreamSubscription listen(void onData(event),\n+      {Function onError, void onDone(), bool cancelOnError}) {\n+    var sub = _incoming.stream\n+        .listen(onData, onError: onError, cancelOnError: cancelOnError);\n+    _incomingSubscription = new WSocketSubscription(sub, onDone, onCancel: () {\n+      _incomingSubscription = null;","path":"lib/src/web_socket/common/w_socket.dart","position":151,"original_position":151,"commit_id":"abd67f36e34cce47efd4f06e2bfc9ac397bd838b","original_commit_id":"abd67f36e34cce47efd4f06e2bfc9ac397bd838b","user":{"login":"trentgrover-wf","id":3778206,"avatar_url":"https://avatars.githubusercontent.com/u/3778206?v=3","gravatar_id":"","url":"https://api.github.com/users/trentgrover-wf","html_url":"https://github.com/trentgrover-wf","followers_url":"https://api.github.com/users/trentgrover-wf/followers","following_url":"https://api.github.com/users/trentgrover-wf/following{/other_user}","gists_url":"https://api.github.com/users/trentgrover-wf/gists{/gist_id}","starred_url":"https://api.github.com/users/trentgrover-wf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/trentgrover-wf/subscriptions","organizations_url":"https://api.github.com/users/trentgrover-wf/orgs","repos_url":"https://api.github.com/users/trentgrover-wf/repos","events_url":"https://api.github.com/users/trentgrover-wf/events{/privacy}","received_events_url":"https://api.github.com/users/trentgrover-wf/received_events","type":"User","site_admin":false},"body":"does the subscription need to be cancelled here too?","created_at":"2016-06-06T16:14:07Z","updated_at":"2016-06-06T16:14:07Z","html_url":"https://github.com/Workiva/w_transport/pull/142#discussion_r65920032","pull_request_url":"https://api.github.com/repos/Workiva/w_transport/pulls/142","_links":{"self":{"href":"https://api.github.com/repos/Workiva/w_transport/pulls/comments/65920032"},"html":{"href":"https://github.com/Workiva/w_transport/pull/142#discussion_r65920032"},"pull_request":{"href":"https://api.github.com/repos/Workiva/w_transport/pulls/142"}}},"pull_request":{"url":"https://api.github.com/repos/Workiva/w_transport/pulls/142","id":72378198,"html_url":"https://github.com/Workiva/w_transport/pull/142","diff_url":"https://github.com/Workiva/w_transport/pull/142.diff","patch_url":"https://github.com/Workiva/w_transport/pull/142.patch","issue_url":"https://api.github.com/repos/Workiva/w_transport/issues/142","number":142,"state":"open","locked":false,"title":"Refactor WSocket to fulfill Stream and StreamSink contracts.","user":{"login":"evanweible-wf","id":1738457,"avatar_url":"https://avatars.githubusercontent.com/u/1738457?v=3","gravatar_id":"","url":"https://api.github.com/users/evanweible-wf","html_url":"https://github.com/evanweible-wf","followers_url":"https://api.github.com/users/evanweible-wf/followers","following_url":"https://api.github.com/users/evanweible-wf/following{/other_user}","gists_url":"https://api.github.com/users/evanweible-wf/gists{/gist_id}","starred_url":"https://api.github.com/users/evanweible-wf/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/evanweible-wf/subscriptions","organizations_url":"https://api.github.com/users/evanweible-wf/orgs","repos_url":"https://api.github.com/users/evanweible-wf/repos","events_url":"https://api.github.com/users/evanweible-wf/events{/privacy}","received_events_url":"https://api.github.com/users/evanweible-wf/received_events","type":"User","site_admin":false},"body":"_Fixes #134._\r\n\r\n## Issue\r\n- `WSocket` extends `Stream` and `StreamSink`, but was not fulfilling those contracts in all scenarios. In particular:\r\n  - After obtaining a `StreamSubscription` instance from a call to `WSocket.listen()`, reassigning the `onData()`, `onError()`, and `onDone()` handlers had no effect.\r\n\r\n      ```dart\r\n      var webSocket = await WSocket.connect(...);\r\n      var subscription = webSocket.listen((data) { ... });\r\n\r\n      // This does nothing:\r\n      subscription.onData((data) { ... });\r\n      // Same goes for onError() and onDone()\r\n      ```\r\n\r\n  - A subscription to a `WSocket` instance did not properly respect pause and resume signals. This could produce a memory leak by buffering WebSocket events indefinitely.\r\n\r\n  - A `WSocket` instance was immediately listening to the underlying WebSocket and buffering events from the underlying WebSocket until a listener was registered. This is not how a standard Dart `Stream` works.\r\n\r\n  - The SockJS configuration was not properly handling the fact that the SockJS `Client` produces WebSocket events with a broadcast stream.\r\n\r\n## Solution\r\n\r\n- The `WSocket` implementation has been refactored such that it behaves exactly like a standard Dart `Stream` and `StreamSink`. This involved several things:\r\n\r\n  - Proxying the subscription to the underlying WebSocket so that the `onDone()`, `onData()`, and `onError()` handlers could be properly reassigned via the `StreamSubscription` api. This was necessary because the `WSocket` instance is not considered \"done\" until both the outgoing and the incoming streams are finished closing. Just relying on the \"done\" event from the subscription to the incoming stream would be inadequate and may fire earlier than desired.\r\n\r\n  - Respecting the state of the subscription to the `WSocket` instance. If it is paused, events from the underlying WebSocket are discarded to prevent buffering of events indefinitely which would cause a memory leak. This is in line with how standard Dart streams work - if the subscription is paused, no events should be added to the stream.\r\n\r\n  - Handling the fact that the SockJS `Client` produces events from a single broadcast stream. This is different than the native `WebSocket` instances because they are single-subscription streams by default. To account for this, the `WSocket` instance waits to subscribe to the broadcast stream until the `onListen` event. Then if the subscription to the `WSocket` instance is paused, the subscription to the underlying SockJS stream is canceled. Finally, if a paused subscription to the `WSocket` instance is resumed, we re-subscribe to the SockJS stream. This is the recommended solution for dealing with subscriptions to broadcast streams.\r\n\r\n- **All of the issues mentioned in the \"Issues\" section have been addressed, and every `WSocket` instance should now behave exactly as a standard `Stream` and `StreamSink` would, regardless of the platform (VM, browser, SockJS, or mock).**\r\n\r\n## Testing\r\n- [ ] CI passes.\r\n- [ ] WebSocket examples still work as expected.\r\n\r\n## Code Review\r\n@trentgrover-wf \r\n@maxwellpeterson-wf \r\n@dustinlessard-wf \r\n@sebastianmalysa-wf \r\n@jayudey-wf \r\n\r\nfyi: @stevenosborne-wf @tayloranderson-wf ","created_at":"2016-06-02T16:35:58Z","updated_at":"2016-06-06T16:14:07Z","closed_at":null,"merged_at":null,"merge_commit_sha":"d23805e8c1ef68d45de939ba17b1f4518c78f39a","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/Workiva/w_transport/pulls/142/commits","review_comments_url":"https://api.github.com/repos/Workiva/w_transport/pulls/142/comments","review_comment_url":"https://api.github.com/repos/Workiva/w_transport/pulls/comments{/number}","comments_url":"https://api.github.com/repos/Workiva/w_transport/issues/142/comments","statuses_url":"https://api.github.com/repos/Workiva/w_transport/statuses/abd67f36e34cce47efd4f06e2bfc9ac397bd838b","head":{"label":"Workiva:ws-rework","ref":"ws-rework","sha":"abd67f36e34cce47efd4f06e2bfc9ac397bd838b","user":{"login":"Workiva","id":1087529,"avatar_url":"https://avatars.githubusercontent.com/u/1087529?v=3","gravatar_id":"","url":"https://api.github.com/users/Workiva","html_url":"https://github.com/Workiva","followers_url":"https://api.github.com/users/Workiva/followers","following_url":"https://api.github.com/users/Workiva/following{/other_user}","gists_url":"https://api.github.com/users/Workiva/gists{/gist_id}","starred_url":"https://api.github.com/users/Workiva/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Workiva/subscriptions","organizations_url":"https://api.github.com/users/Workiva/orgs","repos_url":"https://api.github.com/users/Workiva/repos","events_url":"https://api.github.com/users/Workiva/events{/privacy}","received_events_url":"https://api.github.com/users/Workiva/received_events","type":"Organization","site_admin":false},"repo":{"id":32808289,"name":"w_transport","full_name":"Workiva/w_transport","owner":{"login":"Workiva","id":1087529,"avatar_url":"https://avatars.githubusercontent.com/u/1087529?v=3","gravatar_id":"","url":"https://api.github.com/users/Workiva","html_url":"https://github.com/Workiva","followers_url":"https://api.github.com/users/Workiva/followers","following_url":"https://api.github.com/users/Workiva/following{/other_user}","gists_url":"https://api.github.com/users/Workiva/gists{/gist_id}","starred_url":"https://api.github.com/users/Workiva/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Workiva/subscriptions","organizations_url":"https://api.github.com/users/Workiva/orgs","repos_url":"https://api.github.com/users/Workiva/repos","events_url":"https://api.github.com/users/Workiva/events{/privacy}","received_events_url":"https://api.github.com/users/Workiva/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Workiva/w_transport","description":"","fork":false,"url":"https://api.github.com/repos/Workiva/w_transport","forks_url":"https://api.github.com/repos/Workiva/w_transport/forks","keys_url":"https://api.github.com/repos/Workiva/w_transport/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Workiva/w_transport/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Workiva/w_transport/teams","hooks_url":"https://api.github.com/repos/Workiva/w_transport/hooks","issue_events_url":"https://api.github.com/repos/Workiva/w_transport/issues/events{/number}","events_url":"https://api.github.com/repos/Workiva/w_transport/events","assignees_url":"https://api.github.com/repos/Workiva/w_transport/assignees{/user}","branches_url":"https://api.github.com/repos/Workiva/w_transport/branches{/branch}","tags_url":"https://api.github.com/repos/Workiva/w_transport/tags","blobs_url":"https://api.github.com/repos/Workiva/w_transport/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Workiva/w_transport/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Workiva/w_transport/git/refs{/sha}","trees_url":"https://api.github.com/repos/Workiva/w_transport/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Workiva/w_transport/statuses/{sha}","languages_url":"https://api.github.com/repos/Workiva/w_transport/languages","stargazers_url":"https://api.github.com/repos/Workiva/w_transport/stargazers","contributors_url":"https://api.github.com/repos/Workiva/w_transport/contributors","subscribers_url":"https://api.github.com/repos/Workiva/w_transport/subscribers","subscription_url":"https://api.github.com/repos/Workiva/w_transport/subscription","commits_url":"https://api.github.com/repos/Workiva/w_transport/commits{/sha}","git_commits_url":"https://api.github.com/repos/Workiva/w_transport/git/commits{/sha}","comments_url":"https://api.github.com/repos/Workiva/w_transport/comments{/number}","issue_comment_url":"https://api.github.com/repos/Workiva/w_transport/issues/comments{/number}","contents_url":"https://api.github.com/repos/Workiva/w_transport/contents/{+path}","compare_url":"https://api.github.com/repos/Workiva/w_transport/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Workiva/w_transport/merges","archive_url":"https://api.github.com/repos/Workiva/w_transport/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Workiva/w_transport/downloads","issues_url":"https://api.github.com/repos/Workiva/w_transport/issues{/number}","pulls_url":"https://api.github.com/repos/Workiva/w_transport/pulls{/number}","milestones_url":"https://api.github.com/repos/Workiva/w_transport/milestones{/number}","notifications_url":"https://api.github.com/repos/Workiva/w_transport/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Workiva/w_transport/labels{/name}","releases_url":"https://api.github.com/repos/Workiva/w_transport/releases{/id}","deployments_url":"https://api.github.com/repos/Workiva/w_transport/deployments","created_at":"2015-03-24T15:43:40Z","updated_at":"2016-05-02T15:49:19Z","pushed_at":"2016-06-03T17:11:36Z","git_url":"git://github.com/Workiva/w_transport.git","ssh_url":"git@github.com:Workiva/w_transport.git","clone_url":"https://github.com/Workiva/w_transport.git","svn_url":"https://github.com/Workiva/w_transport","homepage":null,"size":800,"stargazers_count":7,"watchers_count":7,"language":"Dart","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":4,"mirror_url":null,"open_issues_count":15,"forks":4,"open_issues":15,"watchers":7,"default_branch":"master"}},"base":{"label":"Workiva:master","ref":"master","sha":"b378fc40e9753468d1ea0e8dba8230f0518865ea","user":{"login":"Workiva","id":1087529,"avatar_url":"https://avatars.githubusercontent.com/u/1087529?v=3","gravatar_id":"","url":"https://api.github.com/users/Workiva","html_url":"https://github.com/Workiva","followers_url":"https://api.github.com/users/Workiva/followers","following_url":"https://api.github.com/users/Workiva/following{/other_user}","gists_url":"https://api.github.com/users/Workiva/gists{/gist_id}","starred_url":"https://api.github.com/users/Workiva/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Workiva/subscriptions","organizations_url":"https://api.github.com/users/Workiva/orgs","repos_url":"https://api.github.com/users/Workiva/repos","events_url":"https://api.github.com/users/Workiva/events{/privacy}","received_events_url":"https://api.github.com/users/Workiva/received_events","type":"Organization","site_admin":false},"repo":{"id":32808289,"name":"w_transport","full_name":"Workiva/w_transport","owner":{"login":"Workiva","id":1087529,"avatar_url":"https://avatars.githubusercontent.com/u/1087529?v=3","gravatar_id":"","url":"https://api.github.com/users/Workiva","html_url":"https://github.com/Workiva","followers_url":"https://api.github.com/users/Workiva/followers","following_url":"https://api.github.com/users/Workiva/following{/other_user}","gists_url":"https://api.github.com/users/Workiva/gists{/gist_id}","starred_url":"https://api.github.com/users/Workiva/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Workiva/subscriptions","organizations_url":"https://api.github.com/users/Workiva/orgs","repos_url":"https://api.github.com/users/Workiva/repos","events_url":"https://api.github.com/users/Workiva/events{/privacy}","received_events_url":"https://api.github.com/users/Workiva/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/Workiva/w_transport","description":"","fork":false,"url":"https://api.github.com/repos/Workiva/w_transport","forks_url":"https://api.github.com/repos/Workiva/w_transport/forks","keys_url":"https://api.github.com/repos/Workiva/w_transport/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Workiva/w_transport/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Workiva/w_transport/teams","hooks_url":"https://api.github.com/repos/Workiva/w_transport/hooks","issue_events_url":"https://api.github.com/repos/Workiva/w_transport/issues/events{/number}","events_url":"https://api.github.com/repos/Workiva/w_transport/events","assignees_url":"https://api.github.com/repos/Workiva/w_transport/assignees{/user}","branches_url":"https://api.github.com/repos/Workiva/w_transport/branches{/branch}","tags_url":"https://api.github.com/repos/Workiva/w_transport/tags","blobs_url":"https://api.github.com/repos/Workiva/w_transport/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Workiva/w_transport/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Workiva/w_transport/git/refs{/sha}","trees_url":"https://api.github.com/repos/Workiva/w_transport/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Workiva/w_transport/statuses/{sha}","languages_url":"https://api.github.com/repos/Workiva/w_transport/languages","stargazers_url":"https://api.github.com/repos/Workiva/w_transport/stargazers","contributors_url":"https://api.github.com/repos/Workiva/w_transport/contributors","subscribers_url":"https://api.github.com/repos/Workiva/w_transport/subscribers","subscription_url":"https://api.github.com/repos/Workiva/w_transport/subscription","commits_url":"https://api.github.com/repos/Workiva/w_transport/commits{/sha}","git_commits_url":"https://api.github.com/repos/Workiva/w_transport/git/commits{/sha}","comments_url":"https://api.github.com/repos/Workiva/w_transport/comments{/number}","issue_comment_url":"https://api.github.com/repos/Workiva/w_transport/issues/comments{/number}","contents_url":"https://api.github.com/repos/Workiva/w_transport/contents/{+path}","compare_url":"https://api.github.com/repos/Workiva/w_transport/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Workiva/w_transport/merges","archive_url":"https://api.github.com/repos/Workiva/w_transport/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Workiva/w_transport/downloads","issues_url":"https://api.github.com/repos/Workiva/w_transport/issues{/number}","pulls_url":"https://api.github.com/repos/Workiva/w_transport/pulls{/number}","milestones_url":"https://api.github.com/repos/Workiva/w_transport/milestones{/number}","notifications_url":"https://api.github.com/repos/Workiva/w_transport/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Workiva/w_transport/labels{/name}","releases_url":"https://api.github.com/repos/Workiva/w_transport/releases{/id}","deployments_url":"https://api.github.com/repos/Workiva/w_transport/deployments","created_at":"2015-03-24T15:43:40Z","updated_at":"2016-05-02T15:49:19Z","pushed_at":"2016-06-03T17:11:36Z","git_url":"git://github.com/Workiva/w_transport.git","ssh_url":"git@github.com:Workiva/w_transport.git","clone_url":"https://github.com/Workiva/w_transport.git","svn_url":"https://github.com/Workiva/w_transport","homepage":null,"size":800,"stargazers_count":7,"watchers_count":7,"language":"Dart","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":4,"mirror_url":null,"open_issues_count":15,"forks":4,"open_issues":15,"watchers":7,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/Workiva/w_transport/pulls/142"},"html":{"href":"https://github.com/Workiva/w_transport/pull/142"},"issue":{"href":"https://api.github.com/repos/Workiva/w_transport/issues/142"},"comments":{"href":"https://api.github.com/repos/Workiva/w_transport/issues/142/comments"},"review_comments":{"href":"https://api.github.com/repos/Workiva/w_transport/pulls/142/comments"},"review_comment":{"href":"https://api.github.com/repos/Workiva/w_transport/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/Workiva/w_transport/pulls/142/commits"},"statuses":{"href":"https://api.github.com/repos/Workiva/w_transport/statuses/abd67f36e34cce47efd4f06e2bfc9ac397bd838b"}}}}
{"actor":{"display_login":"trentgrover-wf"}}