phergie / phergie-irc-bot-react

IRC bot built on React
BSD 2-Clause "Simplified" License
81 stars 27 forks source link

Connection doesn't time out when using SSL #42

Closed flip111 closed 8 years ago

flip111 commented 8 years ago

there isn't any error messages. My config.php looks like this (nothing special)

<?php
use Phergie\Irc\Connection;

return [
  'plugins' => [
    new \Phergie\Irc\Plugin\React\Pong\Plugin,
    new \Phergie\Irc\Plugin\React\AutoJoin\Plugin([
      'channels' => '#mybot'
    ]),
  ],

  'connections' => [
    new Connection([
      'serverHostname' => 'wilhelm.freenode.net',
      'username' => 'mybot',
      'realname' => 'irc bot',
      'nickname' => 'mybot',
      'serverport' => 6697,
      'options' => [
        'transport' => 'ssl'
      ]
    ]),
  ]
];

with composer.json

{
    "require": {
        "phergie/phergie-irc-bot-react": "^1.3",
        "phergie/phergie-irc-plugin-react-autojoin": "^1.1",
        "phergie/phergie-irc-plugin-react-feedticker": "^1.0",
        "phergie/phergie-irc-plugin-react-nickserv": "^1.3",
        "phergie/phergie-irc-plugin-react-pong": "^1.1",
        "pschwisow/phergie-irc-plugin-react-altnick": "^1.0"
    }
}

so far i tracked the issue down to https://github.com/reactphp/socket-client/blob/v0.4.4/src/Connector.php#L58 it waits for a write event that never comes. There is already a write event before that so i wonder if the asynchronous nature of things causes some timing problems here.

svpernova09 commented 8 years ago

Closing this as not a Phergie issue since it's react not throwing errors when you use the wrong port numbers for the specified transport

flip111 commented 8 years ago

@svpernova09 did you liked to close this issue because you felt you are personally responsible for debugging it? Imo it's ok to leave the issue open, especially since an issue has been filed with the dependency. However it is a phergie issue which seems caused by one of the dependencies. Normally the "parent" issue stays open until a fix in the dependency library and the version has been bumped https://github.com/phergie/phergie-irc-client-react/blob/master/composer.json#L19 By the way, for me it has not entirely be proven yet whether it is indeed a react issue. Also, suppose that react can do a few things without given messages about it phergie could just print to the console a list of possible causes (which would already help the user). Anyway ultimately the decision is yours, but it just strike odd to me as user of the library i experience this issue with phergie

svpernova09 commented 8 years ago

@flip111

The root cause of this issue is that you were using the incorrect port number for an SSL transport.

I agree 100% that there should be something that gives you some feedback.

For your reported problem of "Connection doesn't time out when using SSL" the solution was "use the correct port number for SSL on Freenode servers".

Feel free to open a new issue with suggestions on how we could improve this and we can have that discussion.

Some suggestions I could offer (Ordered by estimated level of effort):

An aside:

I'm sorry if I'm coming across as difficult. I'm genuinely trying to make sure we're aware of root causes and fixes. I'm not a fan of leaving issues open ended. If we're waiting on an upstream fix from a dependency or even a compiler I think we're better served putting that issue in documentation where it makes sense and not letting an open issue rot for months/years.

flip111 commented 8 years ago

I'm hoping to get some response on the react issue i opened, i was planning to come up with a way how to improve this after i actually understand what's going on. So hopefully someone from react can help me understand the code and so can improve this library