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

irc, memories: fix handling QUITs, and prevent infinite failed-connection retries #2306

Merged
merged 4 commits into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ exclude_lines =
if 0:
if False:
if __name__ == .__main__.:

if typing.TYPE_CHECKING:
if TYPE_CHECKING:

dgw marked this conversation as resolved.
Show resolved Hide resolved
show_missing = True

[html]
Expand Down
15 changes: 13 additions & 2 deletions sopel/irc/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def __init__(
verify_ssl: bool = True,
ca_certs: Optional[str] = None,
ssl_ciphers: Optional[List[str]] = None,
ssl_minimum_version: Optional[ssl.TLSVersion] = None,
ssl_minimum_version: ssl.TLSVersion = ssl.TLSVersion.TLSv1_2,
**kwargs,
):
super().__init__(bot)
Expand All @@ -84,7 +84,7 @@ def __init__(
self._keyfile: Optional[str] = keyfile
self._verify_ssl: bool = verify_ssl
self._ca_certs: Optional[str] = ca_certs
self._ssl_ciphers: str = ":".join(ssl_ciphers)
self._ssl_ciphers: str = ":".join(ssl_ciphers or [])
self._ssl_minimum_version: ssl.TLSVersion = ssl_minimum_version

# timeout configuration
Expand Down Expand Up @@ -308,6 +308,17 @@ async def _run_forever(self) -> None:
)
except ssl.SSLError:
LOGGER.exception('Unable to connect due to SSL error.')
# tell the bot to quit without restart
self.bot.hasquit = True
self.bot.wantsrestart = False
return
except Exception:
LOGGER.exception('Unable to connect.')
# until there is a way to prevent an infinite loop of connection
# error and reconnect, we have to tell the bot to quit here
# TODO: prevent infinite connection failure loop
self.bot.hasquit = True
self.bot.wantsrestart = False
return

self._connected = True
Expand Down
2 changes: 1 addition & 1 deletion sopel/plugins/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ def is_limited(
time_limit: datetime.datetime,
) -> bool:
"""Determine if the rule hits the time limit."""
if not self.started_at and not self.ended_at:
if not self.started_at:
dgw marked this conversation as resolved.
Show resolved Hide resolved
# not even started, so not limited
return False

Expand Down
21 changes: 14 additions & 7 deletions sopel/tools/memories.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@

from collections import defaultdict
import threading
from typing import Callable
import typing

from .identifiers import Identifier

if typing.TYPE_CHECKING:
from typing import Callable, Optional

IdentifierFactory = Callable[[str], Identifier]
IdentifierFactory = Callable[[str], Identifier]


class SopelMemory(dict):
Expand Down Expand Up @@ -157,11 +159,16 @@ def __init__(
self.make_identifier = identifier_factory
"""A factory to transform keys into identifiers."""

def __getitem__(self, key):
return super().__getitem__(self.make_identifier(key))
def _make_key(self, key: Optional[str]) -> Optional[Identifier]:
if key is not None:
return self.make_identifier(key)
return None

def __getitem__(self, key: Optional[str]):
return super().__getitem__(self._make_key(key))

def __contains__(self, key):
return super().__contains__(self.make_identifier(key))
return super().__contains__(self._make_key(key))

def __setitem__(self, key, value):
super().__setitem__(self.make_identifier(key), value)
def __setitem__(self, key: Optional[str], value):
super().__setitem__(self._make_key(key), value)
39 changes: 38 additions & 1 deletion test/test_bot.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
"""Tests for core ``sopel.bot`` module"""
from __future__ import annotations

from datetime import datetime, timezone
from datetime import datetime, timedelta, timezone
import re
import typing

import pytest

Expand All @@ -12,6 +13,12 @@
from sopel.tools import Identifier, SopelMemory, target


if typing.TYPE_CHECKING:
from sopel.config import Config
from sopel.tests.factories import BotFactory, IRCFactory, UserFactory
from sopel.tests.mocks import MockIRCServer


TMP_CONFIG = """
[core]
owner = testnick
Expand Down Expand Up @@ -1174,6 +1181,9 @@ def url_handler(*args, **kwargs):
assert not results, "Manually registered callback must not be found"


# -----------------------------------------------------------------------------
# Test various message handling

def test_ignore_replay_servertime(mockbot):
"""Test ignoring messages sent before bot joined a channel."""
@plugin.rule("$nickname!")
Expand Down Expand Up @@ -1201,3 +1211,30 @@ def ping(bot, trigger):
"@time=2021-06-01T12:00:00.020Z :user2!user2@user PRIVMSG #test :TestBot!"
)
assert mockbot.backend.message_sent == rawlist("PRIVMSG #test :user2!")


def test_user_quit(
tmpconfig: Config,
botfactory: BotFactory,
ircfactory: IRCFactory,
userfactory: UserFactory,
):
"""Test the behavior of a QUIT message from another user."""
mockbot: bot.Sopel = botfactory.preloaded(tmpconfig)
server: MockIRCServer = ircfactory(mockbot, True)
server.channel_joined('#test', ['MrPraline'])
mockbot.backend.clear_message_sent()

mockuser = userfactory('MrPraline', 'praline', 'example.com')

assert 'MrPraline' in mockbot.channels['#test'].users

servertime = datetime.utcnow() + timedelta(seconds=10)
mockbot.on_message(
"@time={servertime} :{user} QUIT :Ping timeout: 246 seconds".format(
servertime=servertime.strftime('%Y-%m-%dT%H:%M:%SZ'),
user=mockuser.prefix,
)
)

assert 'MrPraline' not in mockbot.channels['#test'].users
5 changes: 5 additions & 0 deletions test/tools/test_tools_memories.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
from sopel.tools import identifiers, memories


def test_sopel_identifier_memory_none():
memory = memories.SopelIdentifierMemory()
assert None not in memory


def test_sopel_identifier_memory_str():
user = identifiers.Identifier('Exirel')
memory = memories.SopelIdentifierMemory()
Expand Down