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

Socket connection errors shouldn't terminate the app, instead it should reconnect #8

Closed
ErickRodrCodes opened this issue Oct 21, 2021 · 3 comments
Labels
researching Researching the Issue for future determination

Comments

@ErickRodrCodes
Copy link

while working on the new ITunes plugin , I have noted that socket connections that terminates doesn't try to reconnect:

TPiTunes 2021-10-21T03:19:43.454Z [info] : onInfo Called {}
TPiTunes 2021-10-21T03:19:43.457Z [info] : RadioStation has some value. Running executor {}
TPiTunes 2021-10-21T03:19:43.468Z [info] : current track is URL track (IITURLTrack). {}
TPiTunes 2021-10-21T03:19:43.666Z [info] : Triggered TP event onSettings {}
TPiTunes 2021-10-21T03:19:43.749Z [info] : Dreamstate Logic - Space Born {}
TPiTunes 2021-10-21T03:19:43.859Z [info] : Artwork for track 'Dreamstate Logic - Space Born' now available in C:\Git-Projects\touchportal-itunes-ts-plugin\output\album_artwork_temp_orig.jpg {}
2021-10-21T03:22:03.767Z : TPiTunes :ERROR: Socket Connection closed
{
  args: [
    '{\n  "errno": -4060,\n  "code": "ENOBUFS",\n  "syscall": "write"\n}'
  ]
}
2021-10-21T03:22:04.268Z : TPiTunes :WARN: Connection closed
TPiTunes 2021-10-21T03:22:09.660Z [info] : Mr FijiWiji - Cynical feat. CoMa (Original Mix) {}
TPiTunes 2021-10-21T03:22:09.911Z [warn] : Placing default art image for "Mr FijiWiji - Cynical feat. CoMa (Original Mix)": Artwork not found {}

that said, you should reconnect here:

process.exit(0);

@spdermn02
Copy link
Owner

will look at putting in some number of retries. or a progressively longer retry wait loop state. But at some point is is nice to just kill the plugin if Touch Portal isn't running anymore, as if Touch portal is started again, it will restart all plugins and now you have 2 plugins connected.

@spdermn02 spdermn02 added the researching Researching the Issue for future determination label Jan 24, 2022
@mpaperno
Copy link
Collaborator

I'm late to the party but wanted to chime in that this simply won't work, at least with TP v3.x.

The main problem is that TP doesn't always shut down plugins correctly on exit. Most of the time it simply closes the socket w/out any closePlugin message at all. (I've gone through this with Reinier, who recreated/confirmed the issue.) This should be fixed in TP v4, but who knows when that will be ready.

In any case, pretty much the only time you're gonna get a network error on a localhost connection is when the server/other side has gone away. Or your whole network stack crashed. In either case it's pointless to try re-connecting blindly, and as mentioned TP will restart the plugin anyway when it restarts, according to the user's settings. There's no way to tell if TP is actually running or not short of examining all the system's running processes, and even then it doesn't tell you if it maybe has actually crashed/hung but not been killed yet.

Even when the shutdown issue is fixed and (if) TP v4 allows remote plugin plugin connections, I think it should still be up to the plugin author to decide what to do in case of an unexpected socket error. Having the option for TP Client to not exit automatically on error will take care of that, perhaps with a way to pass on the original socket error.

-Max

@mpaperno
Copy link
Collaborator

Since #36 the plugin author can now choose how to handle disconnections and socket errors.

If this doesn't satisfy the issue's request, please feel free to re-open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
researching Researching the Issue for future determination
Projects
None yet
Development

No branches or pull requests

3 participants