Skip to content

Commit

Permalink
irc: user-friendly messages on connection error
Browse files Browse the repository at this point in the history
These error are now managed with a more user-friendly message, and the
exception is sent to the exception log file:

* SSL certificate validation error
* SSL generic error
* Connection timeout (socket timeout, not IRC timeout)
* Invalid hostname
* Unable to connect (temporary disconnected, host unreachable, etc.)

If we inspect the OSError's errno we could provide a more fine-grained
error handling.

We could, also, improve the connection retry for certain errors.
  • Loading branch information
Exirel committed Mar 23, 2023
1 parent 9a241e8 commit dcb1d3c
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 5 deletions.
12 changes: 12 additions & 0 deletions sopel/irc/abstract_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from __future__ import annotations

import abc
import logging
from typing import Optional, TYPE_CHECKING

from .utils import safe
Expand All @@ -41,6 +42,17 @@ def __init__(self, bot: AbstractBot):
def is_connected(self) -> bool:
"""Tell if the backend is connected or not."""

def log_exception(self) -> None:
"""Log an exception to ``sopel.exceptions``.
The IRC backend must use this method to log any exception that isn't
caught by the bot itself (i.e. while handling messages), such as
connection errors, SSL errors, etc.
"""
err_log = logging.getLogger('sopel.exceptions')
err_log.exception('Exception in core')
err_log.error('----------------------------------------')

@abc.abstractmethod
def on_irc_error(self, pretrigger: PreTrigger) -> None:
"""Action to perform when the server sends an error event.
Expand Down
74 changes: 69 additions & 5 deletions sopel/irc/backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import asyncio
import logging
import signal
import socket
import ssl
import threading
from typing import Dict, List, Optional, Tuple, TYPE_CHECKING
Expand Down Expand Up @@ -379,6 +380,7 @@ def get_connection_kwargs(self) -> Dict:

async def _run_forever(self) -> None:
self._loop = asyncio.get_running_loop()
connection_kwargs = self.get_connection_kwargs()

# register signal handlers
for quit_signal in QUIT_SIGNALS:
Expand All @@ -389,16 +391,78 @@ async def _run_forever(self) -> None:
# open connection
try:
self._reader, self._writer = await asyncio.open_connection(
**self.get_connection_kwargs(),
**connection_kwargs,
)
except ssl.SSLError:
LOGGER.exception('Unable to connect due to SSL error.')

# SSL Error
except ssl.SSLCertVerificationError as err:
LOGGER.error(
'Unable to connect due to an SSL validation error: %s',
err,
)
self.log_exception()
# tell the bot to quit without restart
self.bot.hasquit = True
self.bot.wantsrestart = False
return
except ssl.SSLError as err:
LOGGER.error('Unable to connect due to an SSL error: %s', err)
self.log_exception()
# tell the bot to quit without restart
self.bot.hasquit = True
self.bot.wantsrestart = False
return

# Specific connection error
except socket.gaierror as err:
LOGGER.error(
'Unable to connect due to invalid IRC server address: %s',
err,
)
LOGGER.warning(
'You should verify that "%s:%s" is the correct address '
'to connect to the IRC server.',
connection_kwargs.get('host'),
connection_kwargs.get('port'),
)
self.log_exception()
# tell the bot to quit without restart
self.bot.hasquit = True
self.bot.wantsrestart = False
return
except Exception:
LOGGER.exception('Unable to connect.')
except TimeoutError as err:
LOGGER.error('Unable to connect due to a timeout: %s', err)
self.log_exception()
# tell the bot to quit without restart
self.bot.hasquit = True
self.bot.wantsrestart = False
return

# Generic connection error (OSError is used for any connection error)
except OSError as err:
LOGGER.error(
'Unable to connect: %s',
err,
)
LOGGER.warning(
'You should verify that "%s:%s" is the correct address '
'to connect to the IRC server.',
connection_kwargs.get('host'),
connection_kwargs.get('port'),
)
self.log_exception()
# tell the bot to quit without restart
self.bot.hasquit = True
self.bot.wantsrestart = False
return

# Unexpected error
except Exception as err:
LOGGER.error(
'Unable to connect due to an unexpected error: %s',
err,
)
self.log_exception()
# 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
Expand Down

0 comments on commit dcb1d3c

Please sign in to comment.