{"action":"created","issue":{"url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder/issues/296","repository_url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder","labels_url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder/issues/296/labels{/name}","comments_url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder/issues/296/comments","events_url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder/issues/296/events","html_url":"https://github.com/mean-expert-official/loopback-sdk-builder/issues/296","id":198845012,"number":296,"title":"socket errors not been handled and not fed into Observables","user":{"login":"mike-packetwerk","id":20678911,"avatar_url":"https://avatars.githubusercontent.com/u/20678911?v=3","gravatar_id":"","url":"https://api.github.com/users/mike-packetwerk","html_url":"https://github.com/mike-packetwerk","followers_url":"https://api.github.com/users/mike-packetwerk/followers","following_url":"https://api.github.com/users/mike-packetwerk/following{/other_user}","gists_url":"https://api.github.com/users/mike-packetwerk/gists{/gist_id}","starred_url":"https://api.github.com/users/mike-packetwerk/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mike-packetwerk/subscriptions","organizations_url":"https://api.github.com/users/mike-packetwerk/orgs","repos_url":"https://api.github.com/users/mike-packetwerk/repos","events_url":"https://api.github.com/users/mike-packetwerk/events{/privacy}","received_events_url":"https://api.github.com/users/mike-packetwerk/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2017-01-05T00:18:29Z","updated_at":"2017-01-05T14:28:07Z","closed_at":null,"body":"#### What type of issue are you creating?\r\n- [x] Bug\r\n- [ ] Enhancement\r\n- [ ] Question\r\n\r\n#### What version of this module are you using?\r\n- [ ] 2.0.10 (Stable)\r\n- [x] 2.1.0-rc.7 (2.1 Release Candidate 7)\r\n- [ ] Other\r\n\r\nWrite other if any:\r\n\r\n#### Please add a description for your issue:\r\nIn flref.ejs you create three new subjects. A single example being:\r\n\r\nhttps://github.com/mean-expert-official/loopback-sdk-builder/blob/master/lib/angular2/shared/models/flref.ejs#L158\r\n\r\nUnfortunately you do not handle any socket errors, so it is possible that these new observables will never emit/complete and certainly not error. I'm not meaning in channel errors (ie error information passed back from the server), but rather local socket client library errors.\r\n\r\nIn my case  a timing issues exists such that the initial socket traffic passes over the long polling method before the socket is upgraded to a websocket. In this case a couple of pull/broadcast messages get emitted but before hitting the wire the socket is upgraded. The socket library sends these packets as polling and the server rejects them as it has already upgraded to websocket method and discarded the original id used by polling.\r\n\r\nAdding\r\n```\r\n    this.socket.on('disconnect', error => sbj.error(error));\r\n```\r\ninto each of the affected functions fixes the problem in the sense that my client code can retry the connection/handle the lack of response. However, I'm not clear of the implications with leaking memory/adding an every increasing number of listeners as the socket is really global to the file.\r\n\r\nBTW, I tried listening to `this.socket.on('error')` but it never fired.\r\n\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder/issues/comments/270656070","html_url":"https://github.com/mean-expert-official/loopback-sdk-builder/issues/296#issuecomment-270656070","issue_url":"https://api.github.com/repos/mean-expert-official/loopback-sdk-builder/issues/296","id":270656070,"user":{"login":"jonathan-casarrubias","id":1533239,"avatar_url":"https://avatars.githubusercontent.com/u/1533239?v=3","gravatar_id":"","url":"https://api.github.com/users/jonathan-casarrubias","html_url":"https://github.com/jonathan-casarrubias","followers_url":"https://api.github.com/users/jonathan-casarrubias/followers","following_url":"https://api.github.com/users/jonathan-casarrubias/following{/other_user}","gists_url":"https://api.github.com/users/jonathan-casarrubias/gists{/gist_id}","starred_url":"https://api.github.com/users/jonathan-casarrubias/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jonathan-casarrubias/subscriptions","organizations_url":"https://api.github.com/users/jonathan-casarrubias/orgs","repos_url":"https://api.github.com/users/jonathan-casarrubias/repos","events_url":"https://api.github.com/users/jonathan-casarrubias/events{/privacy}","received_events_url":"https://api.github.com/users/jonathan-casarrubias/received_events","type":"User","site_admin":false},"created_at":"2017-01-05T14:28:07Z","updated_at":"2017-01-05T14:28:07Z","body":"Ok, but you have I think at least 3 problems remaining:\r\n\r\n*) in the disconnect handler, you broadcast the disconnect message before actually processing the disconnect. If I wanted to `retry` the onReady it creates an infinite loop. ie I think the code should be:\r\n\r\n```\r\n  this.disconnect();\r\n  connectionSubject.error('disconnected');\r\n```\r\n\r\n**Ok that make sense, I will try to move it before firing the vent.**\r\n\r\n*) I think you have a kind of race condition with your `if (this.connected)` statement. Between calling `onReady` and the connection being established (the `connect` event) is enough time for many additional `onReady` calls, all which will create new `new FireLoop(this.socket, this.models);` The result is the cache of FileLoopRefs is not globally unique.\r\n\r\n**I think this can be solved by adding a new status \"connecting\" and wait until it is actually connected o there are no multiple FireLoop and connections**\r\n\r\n*) The original problem I reported above still remains. ie calls into FileLoopRef might never return (emit/errror/complete). It seems unnecessarily messy to ask uses of FileLoopRef to have complex Observable pipelines needing to track `RealTime.onDisconnect` with say `FileLoopRef.on` simultaneously. Actually, I'm not sure it is possible to get it right. I would be happy to see you directly address this point, and find my logic hole.\r\n\r\nI'm not totally agree with you on this one, because decoupling functionality to later be used with other modules it does not seems messy for me.\r\n\r\nThe problem with this project is that it manage several use cases and people asking requests only take care for their specific use case, as I said there are 3 different ways to communicate through websockets using the SDK Builder and all of them share this issue. You are focusing in your personal specific issue, but you are not taking in mind that doing the way you are asking, would not need not to duplicate but triplicate code.\r\n\r\nSo yes, making FireLoop references to handle all of these issues would fix your problem, but would lead to many problems to other persons.\r\n\r\nAnyway, it is in my plans to separate the SDKs once the moment arrive"}}
{"actor":{"display_login":"jonathan-casarrubias"}}