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

Critical error hidden in logging type INFO #14367

Open
aceArt-GmbH opened this issue Nov 4, 2022 · 3 comments
Open

Critical error hidden in logging type INFO #14367

aceArt-GmbH opened this issue Nov 4, 2022 · 3 comments
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@aceArt-GmbH
Copy link
Contributor

aceArt-GmbH commented Nov 4, 2022

Description

I was trying to add an event_persister worker via docker-compose using synapse_worker_event_persister1 as service name which also is the docker DNS record.
But apparently _ is not allowed for event_persister host's (which IMO is a bug on its on).

The error was only visible when showing "INFO" logs

Steps to reproduce

  • add a generic worker in docker-compose.yaml with synapse_worker_event_persister1 as name
  • reference it
instance_map:
    event_persister1:
        host: synapse_worker_event_persister1
        port: 8034
  • send requests and get M_UNKNOWN: Failed to talk to event_persister1 process
  • show docker logs and see nothing
version: 1

formatters:
  precise:
    format: '%(asctime)s - %(name)s - %(lineno)d - %(levelname)s - %(request)s - %(message)s'

handlers:


  console:
    class: logging.StreamHandler
    formatter: precise

loggers:
    synapse.storage.SQL:
        # beware: increasing this to DEBUG will make synapse log sensitive
        # information such as access tokens.
        level: INFO

root:
    level: WARN


    handlers: [console]


disable_existing_loggers: false
  • change root.level to INFO to finally see error

Homeserver

self hosted

Synapse Version

1.70.1

Installation Method

Docker (matrixdotorg/synapse)

Platform

linux

Relevant log output

2022-11-04 14:19:14,311 - synapse.http.client - 455 - INFO - POST-5 - Error sending request to POST http://synapse_worker_event_persister1:8034/_synapse/replication/send_events/JfZHQlavDY: InvalidCodepoint Codepoint U+005F at position 8 of 'synapse_worker_event_persister1' not allowed

Anything else that would be useful to know?

I would guess

raise SynapseError(
502, f"Failed to talk to {instance_name} process"
) from e
is the code throwing the error

@squahtx squahtx added A-Logging Synapse's logs (structured or otherwise). Not metrics. S-Minor Blocks non-critical functionality, workarounds exist. O-Uncommon Most users are unlikely to come across this or unexpected workflow T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Nov 7, 2022
@DMRobertson
Copy link
Contributor

Hostnames are not allowed to contain underscores, see e.g. man 7 hostname, Wikipedia

RFC1912 clarifies:

The presence
of underscores in a label is allowed in [RFC 1033], except [RFC 1033]
is informational only and was not defining a standard. There is at
least one popular TCP/IP implementation which currently refuses to
talk to hosts named with underscores in them.

You should replace your underscores with dashes.


AFAICS the reported error comes from here where a library calls idna.encode. In turn, this is called by treq. I think this is perfectly sensible.

Action here for us:

  • log inter-worker communication failures as errors
  • maybe: check hostnames are valid in config at startup (using idna.encode ourselves?)

@aceArt-GmbH
Copy link
Contributor Author

Hostnames are not allowed to contain underscores

That may be. But why even check for it?
Docker-compose allows using _, DNS does and other tools like curl also don't care. So unless the used library is unable to handle such names, I would just blindly pass on what is given even if it is against the spec

@DMRobertson
Copy link
Contributor

So unless the used library is unable to handle such names

Ultimately this is a choice made by our (transitive) dependency idna; the logic is at https://github.com/kjd/idna/blob/37c7d9b25b8d628f6a14d7b55ffe2c534a225a69/idna/core.py#L308-L337. Synapse-the-project has no strong opinions about what constitutes a valid hostname.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Logging Synapse's logs (structured or otherwise). Not metrics. O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

3 participants