Commit e5ac5e9b authored by Philipp Hörist's avatar Philipp Hörist

Rework message error handling

- Database: Save error in the corresponding message datarow
- ConversationTextview: Show error icon with error message as tooltip
parent e1c684c6
......@@ -46,6 +46,7 @@ from gajim.common.i18n import _
from gajim.common.helpers import AdditionalDataDict
from gajim.common.helpers import open_uri
from gajim.common.helpers import geo_provider_from_location
from gajim.common.helpers import event_filter
from gajim.common.contacts import GC_Contact
from gajim.common.const import AvatarSize
from gajim.common.const import KindConstant
......@@ -231,12 +232,20 @@ class ChatControl(ChatControlBase):
app.ged.register_event_handler(
'receipt-received',
ged.GUI1, self._receipt_received)
app.ged.register_event_handler('message-error',
ged.GUI1, self._on_message_error)
app.ged.register_event_handler('zeroconf-error', ged.GUI1,
self._on_zeroconf_error)
# PluginSystem: adding GUI extension point for this ChatControl
# instance object
app.plugin_manager.gui_extension_point('chat_control', self)
self.update_actions()
@property
def jid(self):
return self.contact.jid
def add_actions(self):
super().add_actions()
actions = [
......@@ -824,6 +833,10 @@ class ChatControl(ChatControlBase):
== pw.get_active_control() and pw.is_active() and end):
app.logger.set_read_messages([obj.msg_log_id])
@event_filter(['account', 'jid'])
def _on_message_error(self, event):
self.conv_textview.show_error(event.message_id, event.error)
def _message_sent(self, obj):
if obj.conn.name != self.account:
return
......@@ -880,7 +893,7 @@ class ChatControl(ChatControlBase):
def add_message(self, text, frm='', tim=None, encrypted=None,
subject=None, xhtml=None,
displaymarking=None, msg_log_id=None, correct_id=None,
message_id=None, additional_data=None):
message_id=None, additional_data=None, error=None):
"""
Print a line in the conversation
......@@ -922,7 +935,7 @@ class ChatControl(ChatControlBase):
displaymarking=displaymarking,
msg_log_id=msg_log_id, message_id=message_id,
correct_id=correct_id, additional_data=additional_data,
encrypted=encrypted)
encrypted=encrypted, error=error)
if text.startswith('/me ') or text.startswith('/me\n'):
self.old_msg_kind = None
else:
......@@ -1252,7 +1265,8 @@ class ChatControl(ChatControlBase):
restored=True,
old_kind=local_old_kind,
xhtml=xhtml,
additional_data=additional_data)
additional_data=additional_data,
error=row.error)
if row.message.startswith('/me ') or row.message.startswith('/me\n'):
local_old_kind = None
else:
......@@ -1355,6 +1369,10 @@ class ChatControl(ChatControlBase):
'name': name, 'show': uf_show, 'status': status or ''}
self.add_status_message(status_line)
@event_filter(['account', 'jid'])
def _on_zeroconf_error(self, event):
self.add_status_message(event.message)
def _info_bar_show_message(self):
if self.info_bar.get_visible():
# A message is already shown
......
......@@ -991,7 +991,7 @@ class ChatControlBase(MessageControl, ChatCommandProcessor, CommandTools):
old_kind=None, xhtml=None,
displaymarking=None, msg_log_id=None,
message_id=None, correct_id=None, additional_data=None,
encrypted=None):
encrypted=None, error=None):
"""
Print 'chat' type messages
correct_id = (message_id, correct_id)
......@@ -1017,7 +1017,7 @@ class ChatControlBase(MessageControl, ChatCommandProcessor, CommandTools):
subject, old_kind, xhtml,
displaymarking=displaymarking, message_id=message_id,
correct_id=correct_id, additional_data=additional_data,
encrypted=encrypted)
encrypted=encrypted, error=error)
if restored:
return
......
......@@ -44,9 +44,6 @@ class ConnectionHandlersBase:
# keep track of sessions this connection has with other JIDs
self.sessions = {}
# IDs of sent messages (https://trac.gajim.org/ticket/8222)
self.sent_message_ids = []
def get_sessions(self, jid):
"""
Get all sessions for the given full jid
......
......@@ -58,30 +58,6 @@ class OurShowEvent(nec.NetworkIncomingEvent):
class MessageSentEvent(nec.NetworkIncomingEvent):
name = 'message-sent'
def generate(self):
if not self.automatic_message:
self.conn.sent_message_ids.append(self.stanza_id)
# only record the last 20000 message ids (should be about 1MB [36 byte per uuid]
# and about 24 hours if you send out a message every 5 seconds)
self.conn.sent_message_ids = self.conn.sent_message_ids[-20000:]
return True
class MessageErrorEvent(nec.NetworkIncomingEvent):
name = 'message-error'
def init(self):
self.zeroconf = False
def generate(self):
if self.zeroconf:
return True
self.id_ = self.stanza.getID()
#only alert for errors of explicitly sent messages (see https://trac.gajim.org/ticket/8222)
if self.id_ in self.conn.sent_message_ids:
self.conn.sent_message_ids.remove(self.id_)
return True
return False
class JingleRequestReceivedEvent(nec.NetworkIncomingEvent):
name = 'jingle-request-received'
......
......@@ -40,6 +40,7 @@ from gi.repository import GLib
from nbxmpp.protocol import Node
from nbxmpp.protocol import Iq
from nbxmpp.structs import DiscoInfo
from nbxmpp.structs import CommonError
from nbxmpp.modules.dataforms import extend_form
from nbxmpp.modules.discovery import parse_disco_info
......@@ -76,6 +77,7 @@ LOGS_SQL_STATEMENT = '''
kind INTEGER,
show INTEGER,
message TEXT,
error TEXT,
subject TEXT,
additional_data TEXT,
stanza_id TEXT,
......@@ -93,7 +95,7 @@ LOGS_SQL_STATEMENT = '''
);
CREATE INDEX idx_logs_jid_id_time ON logs (jid_id, time DESC);
CREATE INDEX idx_logs_stanza_id ON logs (stanza_id);
PRAGMA user_version=2;
PRAGMA user_version=4;
'''
CACHE_SQL_STATEMENT = '''
......@@ -187,10 +189,18 @@ def _convert_disco_info(disco_info):
def _adapt_disco_info(disco_info):
return str(disco_info.stanza)
sqlite.register_converter('disco_info', _convert_disco_info)
def _convert_common_error(common_error):
return CommonError.from_string(common_error)
def _adapt_common_error(common_error):
return common_error.serialize()
sqlite.register_converter('disco_info', _convert_disco_info)
sqlite.register_adapter(DiscoInfo, _adapt_disco_info)
sqlite.register_converter('common_error', _convert_common_error)
sqlite.register_adapter(CommonError, _adapt_common_error)
class Logger:
def __init__(self):
......@@ -306,6 +316,13 @@ class Logger:
]
self._execute_multiple(con, statements)
if self._get_user_version(con) < 4:
statements = [
'ALTER TABLE logs ADD COLUMN "error" TEXT',
'PRAGMA user_version=4'
]
self._execute_multiple(con, statements)
def _migrate_cache(self, con):
if self._get_user_version(con) == 0:
# All migrations from 0.16.9 until 1.0.0
......@@ -781,7 +798,8 @@ class Logger:
jids = self._get_family_jids(account, jid)
sql = '''
SELECT time, kind, message, subject, additional_data
SELECT time, kind, message, error as "error [common_error]",
subject, additional_data
FROM logs NATURAL JOIN jids WHERE jid IN ({jids}) AND
kind IN ({kinds}) AND time > get_timeout()
ORDER BY time DESC, log_line_id DESC LIMIT ? OFFSET ?
......@@ -1434,6 +1452,34 @@ class Logger:
return lastrowid
def set_message_error(self, account_jid, jid, message_id, error):
"""
Update the corresponding message with the error
:param account_jid: The jid of the account
:param jid: The jid that belongs to the avatar
:param message_id: The id of the message
:param error: The error stanza as string
"""
account_id = self.get_jid_id(account_jid)
try:
jid_id = self.get_jid_id(str(jid))
except ValueError:
# Unknown JID
return
sql = '''
UPDATE logs SET error = ?
WHERE account_id = ? AND jid_id = ? AND message_id = ?
'''
self._con.execute(sql, (error, account_id, jid_id, message_id))
self._timeout_commit()
def set_avatar_sha(self, account_jid, jid, sha=None):
"""
Set the avatar sha of a jid on an account
......
......@@ -18,10 +18,8 @@ import time
import nbxmpp
from nbxmpp.structs import StanzaHandler
from nbxmpp.const import MessageType
from gajim.common import app
from gajim.common.i18n import _
from gajim.common.nec import NetworkEvent
from gajim.common.helpers import AdditionalDataDict
from gajim.common.const import KindConstant
......@@ -31,7 +29,6 @@ from gajim.common.modules.security_labels import parse_securitylabel
from gajim.common.modules.misc import parse_correction
from gajim.common.modules.misc import parse_oob
from gajim.common.modules.misc import parse_xhtml
from gajim.common.connection_handlers_events import MessageErrorEvent
class Message(BaseModule):
......@@ -42,6 +39,10 @@ class Message(BaseModule):
StanzaHandler(name='message',
callback=self._message_received,
priority=50),
StanzaHandler(name='message',
typ='error',
callback=self._message_error_received,
priority=50),
]
# XEPs for which this message module should not be executed
......@@ -49,7 +50,9 @@ class Message(BaseModule):
nbxmpp.NS_IBB])
def _message_received(self, _con, stanza, properties):
if properties.is_mam_message or properties.is_pubsub:
if (properties.is_mam_message or
properties.is_pubsub or
properties.type.is_error):
return
# Check if a child of the message contains any
# namespaces that we handle in other modules.
......@@ -108,19 +111,6 @@ class Message(BaseModule):
if not gc_control:
minimized = app.interface.minimized_controls[self._account]
gc_control = minimized.get(jid)
if gc_control and jid == fjid:
if properties.type.is_error:
if msgtxt:
msgtxt = _('error while sending %(message)s ( %(error)s )') % {
'message': msgtxt, 'error': stanza.getErrorMsg()}
else:
msgtxt = _('error: %s') % stanza.getErrorMsg()
# TODO: why is this here?
if stanza.getTag('html'):
stanza.delChild('html')
type_ = MessageType.GROUPCHAT
session = None
if not properties.type.is_groupchat:
if properties.is_muc_pm and properties.type.is_error:
......@@ -158,29 +148,6 @@ class Message(BaseModule):
if properties.eme is not None:
msgtxt = get_eme_message(properties.eme)
if type_.is_error:
if not msgtxt:
msgtxt = _('message')
if gc_control:
gc_control.add_info_message(msgtxt)
else:
error_msg = stanza.getErrorMsg() or msgtxt
msgtxt = None if error_msg == msgtxt else msgtxt
self._log_error_message(error_msg,
jid,
properties.timestamp)
app.nec.push_incoming_event(
MessageErrorEvent(None,
conn=self._con,
fjid=fjid,
error_code=stanza.getErrorCode(),
error_msg=error_msg,
msg=msgtxt,
time_=properties.timestamp,
session=session,
stanza=stanza))
return
event_attr = {
'conn': self._con,
'stanza': stanza,
......@@ -234,13 +201,25 @@ class Message(BaseModule):
app.nec.push_incoming_event(
NetworkEvent('decrypted-message-received', **event_attr))
def _log_error_message(self, error_msg, jid, timestamp):
if app.config.should_log(self._account, jid):
app.logger.insert_into_logs(self._account,
jid,
timestamp,
KindConstant.ERROR,
message=error_msg)
def _message_error_received(self, _con, _stanza, properties):
jid = properties.jid
if not properties.is_muc_pm:
jid.setBare()
self._log.info(properties.error)
app.logger.set_message_error(app.get_jid_from_account(self._account),
jid,
properties.id,
properties.error)
app.nec.push_incoming_event(
NetworkEvent('message-error',
account=self._account,
jid=jid,
room_jid=jid,
message_id=properties.id,
error=properties.error))
def _log_muc_message(self, event):
if event.mtype == 'error':
......
......@@ -326,6 +326,9 @@ class P2PClient(IdleObject):
self.RegisterHandler(*StanzaHandler(name='message',
callback=self._caller._messageCB))
self.RegisterHandler(*StanzaHandler(name='message',
typ='error',
callback=self._caller._message_error_received))
self._caller._register_new_handlers(self)
......
......@@ -25,15 +25,12 @@ import logging
from gajim.common import app
from gajim.common import connection_handlers
from gajim.common.i18n import _
from gajim.common.helpers import AdditionalDataDict
from gajim.common.nec import NetworkEvent
from gajim.common.const import KindConstant
from gajim.common.modules.util import get_eme_message
from gajim.common.modules.misc import parse_correction
from gajim.common.modules.misc import parse_oob
from gajim.common.modules.misc import parse_xhtml
from gajim.common.connection_handlers_events import MessageErrorEvent
log = logging.getLogger('gajim.c.z.connection_handlers_zeroconf')
......@@ -50,6 +47,8 @@ class ConnectionHandlersZeroconf(connection_handlers.ConnectionHandlersBase):
"""
Called when we receive a message
"""
if properties.is_error:
return
log.info('Zeroconf MessageCB')
# Dont trust from attr set by sender
......@@ -91,25 +90,6 @@ class ConnectionHandlersZeroconf(connection_handlers.ConnectionHandlersBase):
if properties.eme is not None:
msgtxt = get_eme_message(properties.eme)
if type_ == 'error':
if not msgtxt:
msgtxt = _('message')
self._log_error_message(stanza, msgtxt, jid, timestamp)
error_msg = stanza.getErrorMsg() or msgtxt
msgtxt = None if error_msg == msgtxt else msgtxt
app.nec.push_incoming_event(
MessageErrorEvent(None,
conn=self,
fjid=fjid,
error_code=stanza.getErrorCode(),
error_msg=error_msg,
msg=msgtxt,
time_=timestamp,
session=session,
stanza=stanza))
return
event_attr = {
'conn': self,
'stanza': stanza,
......@@ -146,11 +126,17 @@ class ConnectionHandlersZeroconf(connection_handlers.ConnectionHandlersBase):
app.nec.push_incoming_event(
NetworkEvent('decrypted-message-received', **event_attr))
def _log_error_message(self, stanza, msgtxt, jid, timestamp):
error_msg = stanza.getErrorMsg() or msgtxt
if app.config.should_log(self.name, jid):
app.logger.insert_into_logs(self.name,
jid,
timestamp,
KindConstant.ERROR,
message=error_msg)
def _message_error_received(self, _con, _stanza, properties):
log.info(properties.error)
app.logger.set_message_error(app.get_jid_from_account(self.name),
properties.jid,
properties.id,
properties.error)
app.nec.push_incoming_event(
NetworkEvent('message-error',
account=self.name,
jid=properties.jid,
message_id=properties.id,
error=properties.error))
......@@ -49,7 +49,6 @@ from gajim.common.connection_handlers_events import OurShowEvent
from gajim.common.connection_handlers_events import InformationEvent
from gajim.common.connection_handlers_events import ConnectionLostEvent
from gajim.common.connection_handlers_events import MessageSentEvent
from gajim.common.connection_handlers_events import MessageErrorEvent
log = logging.getLogger('gajim.c.connection_zeroconf')
......@@ -423,9 +422,11 @@ class ConnectionZeroconf(CommonConnection, ConnectionHandlersZeroconf):
def on_send_not_ok(reason):
reason += ' ' + _('Your message could not be sent.')
app.nec.push_incoming_event(MessageErrorEvent(
None, conn=self, fjid=obj.jid, error_code=-1, error_msg=reason,
msg=None, time_=None, session=obj.session, zeroconf=True))
app.nec.push_incoming_event(NetworkEvent(
'zeroconf-error',
account=self.name,
jid=obj.jid,
message=reason))
# Dont propagate event
return True
......@@ -438,10 +439,11 @@ class ConnectionZeroconf(CommonConnection, ConnectionHandlersZeroconf):
# Contact Offline
error_message = _(
'Contact is offline. Your message could not be sent.')
app.nec.push_incoming_event(MessageErrorEvent(
None, conn=self, fjid=obj.jid, error_code=-1,
error_msg=error_message, msg=None, time_=None,
session=obj.session, zeroconf=True))
app.nec.push_incoming_event(NetworkEvent(
'zeroconf-error',
account=self.name,
jid=obj.jid,
message=error_message))
# Dont propagate event
return True
......@@ -457,16 +459,15 @@ class ConnectionZeroconf(CommonConnection, ConnectionHandlersZeroconf):
CommonConnection._event_dispatcher(self, realm, event, data)
if realm == '':
if event == nbxmpp.transports.DATA_ERROR:
thread_id = data[1]
frm = data[0]
session = self.get_or_create_session(frm, thread_id)
error_message = _(
'Connection to host could not be established: '
'Timeout while sending data.')
app.nec.push_incoming_event(MessageErrorEvent(
None, conn=self, fjid=frm, error_code=-1,
error_msg=error_message, msg=None, time_=None,
session=session, zeroconf=True))
app.nec.push_incoming_event(NetworkEvent(
'zeroconf-error',
account=self.name,
jid=frm,
message=error_message))
def cleanup(self):
pass
......@@ -42,7 +42,9 @@ from gajim.common import i18n
from gajim.common.i18n import _
from gajim.common.helpers import AdditionalDataDict
from gajim.common.fuzzyclock import FuzzyClock
from gajim.common.const import StyleAttr, Trust
from gajim.common.const import StyleAttr
from gajim.common.const import Trust
from gajim.common.helpers import to_user_string
from gajim.gtk import util
from gajim.gtk.util import get_cursor
......@@ -407,6 +409,12 @@ class ConversationTextview(GObject.GObject):
return
line.show_receipt_icon()
def show_error(self, id_, error):
line = self._get_message_line(id_)
if line is None:
return
line.show_error_icon(to_user_string(error))
def show_focus_out_line(self):
if not self.allow_focus_out_line:
# if room did not receive focus-in from the last time we added
......@@ -909,7 +917,7 @@ class ConversationTextview(GObject.GObject):
other_tags_for_name=None, other_tags_for_time=None, other_tags_for_text=None,
subject=None, old_kind=None, xhtml=None, graphics=True,
displaymarking=None, message_id=None, correct_id=None, additional_data=None,
encrypted=None):
encrypted=None, error=None):
"""
Print 'chat' type messages
"""
......@@ -1044,6 +1052,9 @@ class ConversationTextview(GObject.GObject):
message_line.show_correction_icon(
self.corrected_text_list[message_id])
if error is not None:
message_line.show_error_icon(to_user_string(error))
if index is None:
# New Message
self._message_list.append(message_line)
......@@ -1311,6 +1322,10 @@ class MessageLine:
self._message_icons.set_correction_icon_visible(True)
self._message_icons.set_correction_tooltip(tooltip)
def show_error_icon(self, tooltip):
self._message_icons.set_error_icon_visible(True)
self._message_icons.set_error_tooltip(tooltip)
class MessageIcons(Gtk.Box):
def __init__(self):
......@@ -1328,8 +1343,14 @@ class MessageIcons(Gtk.Box):
self._receipt_image.set_tooltip_text(_('Received'))
self._receipt_image.set_no_show_all(True)
self._error_image = Gtk.Image.new_from_icon_name(
'dialog-warning-symbolic', Gtk.IconSize.MENU)
self._error_image.get_style_context().add_class('warning-color')
self._error_image.set_no_show_all(True)
self.add(self._correction_image)
self.add(self._receipt_image)
self.add(self._error_image)
self.show_all()
def set_receipt_icon_visible(self, visible):
......@@ -1342,3 +1363,9 @@ class MessageIcons(Gtk.Box):
def set_correction_tooltip(self, text):
self._correction_image.set_tooltip_markup(text)
def set_error_icon_visible(self, visible):
self._error_image.set_visible(visible)
def set_error_tooltip(self, text):
self._error_image.set_tooltip_markup(text)
......@@ -1573,6 +1573,9 @@ class GroupchatControl(ChatControlBase):
self.msg_textview.get_buffer().set_text('')
self.msg_textview.grab_focus()
@event_filter(['account', 'room_jid'])
def _on_message_error(self, event):
self.conv_textview.show_error(event.message_id, event.error)
def minimizable(self):
if self.force_non_minimizable:
......
......@@ -285,61 +285,6 @@ class Interface:
if ctrl and ctrl.session and len(obj.contact_list) > 1:
ctrl.remove_session(ctrl.session)
def handle_event_msgerror(self, obj):
#'MSGERROR' (account, (jid, error_code, error_msg, msg, time[session]))
account = obj.conn.name
jids = obj.fjid.split('/', 1)
jid = jids[0]
session = obj.session
gc_control = self.msg_win_mgr.get_gc_control(jid, account)
if not gc_control and \
jid in self.minimized_controls[account]: