Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

User directory gets stuck when encountering non-string display name #8220

Closed
erikjohnston opened this issue Sep 1, 2020 · 2 comments · Fixed by #8223
Closed

User directory gets stuck when encountering non-string display name #8220

erikjohnston opened this issue Sep 1, 2020 · 2 comments · Fixed by #8223
Assignees

Comments

@erikjohnston
Copy link
Member

erikjohnston commented Sep 1, 2020

Stack trace:

InvalidTextRepresentation: malformed array literal: ""
LINE 1: ...t(to_tsvector('english', COALESCE(ARRAY['...'], '')), 'B')...
                                                             ^
DETAIL:  Array value must start with "{" or dimension information.

  File "synapse/metrics/background_process_metrics.py", line 205, in run
    result = await result
  File "synapse/handlers/user_directory.py", line 103, in process
    await self._unsafe_process()
  File "synapse/handlers/user_directory.py", line 154, in _unsafe_process
    await self._handle_deltas(deltas)
  File "synapse/handlers/user_directory.py", line 217, in _handle_deltas
    state_key, room_id, prev_event_id, event_id
  File "synapse/handlers/user_directory.py", line 396, in _handle_profile_change
    await self.store.update_profile_in_user_dir(user_id, new_name, new_avatar)
  File "twisted/internet/defer.py", line 1416, in _inlineCallbacks
    result = result.throwExceptionIntoGenerator(g)
  File "twisted/python/failure.py", line 512, in throwExceptionIntoGenerator
    return g.throw(self.type, self.value, self.tb)
  File "synapse/storage/database.py", line 541, in runInteraction
    **kwargs
  File "twisted/internet/defer.py", line 1416, in _inlineCallbacks
    result = result.throwExceptionIntoGenerator(g)
  File "twisted/python/failure.py", line 512, in throwExceptionIntoGenerator
    return g.throw(self.type, self.value, self.tb)
  File "synapse/storage/database.py", line 591, in runWithConnection
    self._db_pool.runWithConnection(inner_func, *args, **kwargs)
  File "twisted/python/threadpool.py", line 250, in inContext
    result = inContext.theWork()
  File "twisted/python/threadpool.py", line 266, in <lambda>
    inContext.theWork = lambda: context.call(ctx, func, *args, **kw)
  File "twisted/python/context.py", line 122, in callWithContext
    return self.currentContext().callWithContext(ctx, func, *args, **kw)
  File "twisted/python/context.py", line 85, in callWithContext
    return func(*args,**kw)
  File "twisted/enterprise/adbapi.py", line 306, in _runWithConnection
    compat.reraise(excValue, excTraceback)
  File "twisted/python/compat.py", line 464, in reraise
    raise exception.with_traceback(traceback)
  File "twisted/enterprise/adbapi.py", line 297, in _runWithConnection
    result = func(conn, *args, **kw)
  File "synapse/storage/database.py", line 588, in inner_func
    return func(conn, *args, **kwargs)
  File "synapse/storage/database.py", line 429, in new_transaction
    r = func(cursor, *args, **kwargs)
  File "synapse/storage/databases/main/user_directory.py", line 400, in _update_profile_in_user_dir_txn
    display_name,
  File "synapse/storage/database.py", line 212, in execute
    self._do_execute(self.txn.execute, sql, *args)
  File "synapse/storage/database.py", line 238, in _do_execute
    return func(sql, *args)

Basically we're passing an array into a query that expects it to be a string. We should a) handle this error and b) figure out how to stop it from tight looping trying to handle that event.

(Internal sentry link: https://sentry.matrix.org/sentry/synapse-matrixorg/issues/124595/)

@babolivier babolivier assigned babolivier and unassigned babolivier Sep 1, 2020
@clokep clokep self-assigned this Sep 1, 2020
@clokep
Copy link
Member

clokep commented Sep 1, 2020

There was some conversation around this, summary below:

  • The main issue is that the error path seems to continue throwing errors and stops making progress.
  • Step one is just adding checks to
    prev_name = prev_event.content.get("displayname")
    new_name = event.content.get("displayname")
    prev_avatar = prev_event.content.get("avatar_url")
    new_avatar = event.content.get("avatar_url")

The current fix being used is:

diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py
--- a/synapse/storage/databases/main/user_directory.py
+++ b/synapse/storage/databases/main/user_directory.py
@@ -397,7 +397,7 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
                             user_id,
                             get_localpart_from_id(user_id),
                             get_domain_from_id(user_id),
-                            display_name,
+                            str(display_name),
                         ),
                     )
                 else:

@clokep
Copy link
Member

clokep commented Sep 1, 2020

It was also mentioned that the stats code has a similar background process which likely has a similar bug in it.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants