matrix-nio icon indicating copy to clipboard operation
matrix-nio copied to clipboard

room_create: creator field missing when creating room triggers error when listening room creating event

Open xgenvn opened this issue 2 years ago • 8 comments

Hi,

I use the room_create API, but it seems the creator field not included in the state content.

This is the error when consuming room create event:

On instance['content']:
    {'m.federate': False, 'room_version': '11', 'type': ''}
Error validating event: 'creator' is a required property

Failed validating 'required' in schema['properties']['content']:
    {'properties': {'creator': {'format': 'user_id', 'type': 'string'},
                    'm.federate': {'default': True, 'type': 'boolean'},
                    'predecessor': {'properties': {'event_id': {'type': 'string'},
                                                   'room_id': {'format': 'room_id',
                                                               'type': 'string'}},
                                    'required': ['event_id', 'room_id'],
                                    'type': 'object'},
                    'room_version': {'default': '1', 'type': 'string'},
                    'type': {'default': '', 'type': 'string'}},
     'required': ['creator'],
     'type': 'object'}

There are several options but I'm not sure which one is best:

  1. Remove required from schemas for room_create event. Also creator = parsed_dict["content"].get("creator") in RoomCreateEvent class
  2. Add creator param in room_create API

Thanks and regards.

xgenvn avatar Dec 28 '23 19:12 xgenvn

@PaarthShah do you have time to have a look at the issue? I can create a PR accordingly, but since I'm new to the library and Matrix, I need some instructions.

xgenvn avatar Jan 06 '24 14:01 xgenvn

Hi @xgenvn!

Your first point of reference should be the matrix spec:

https://spec.matrix.org/legacy/r0.0.0/client_server.html#m-room-create

The creator field seems to be expected.

https://spec.matrix.org/legacy/r0.0.0/client_server.html#creation

The homeserver will create an m.room.create event when a room is created, which serves as the root of the event graph for this room. This event also has a creator key which contains the user ID of the room creator.

The use of the word "has" implies its required (but I agree that it's strange that it's not explicit).

https://spec.matrix.org/legacy/r0.0.0/client_server.html#post-matrix-client-r0-createroom

Extra keys to be added to the content of the m.room.create. The server will clober [sic] the following keys: creator. Future versions of the specification may allow the server to clobber other keys.

Overall, I believe the spec says that creator is required, and that nio is behaving correctly. There is also no extra parameter you can/should pass within the create in order to fix it.

However, I have to ask: what homeserver software are you running against? Is this Synapse, Dendrite, Conduit, or something else? Because my primary suspicion is that said homeserver is engaging in non-spec compliant behavior, and I wouldn't expect a major implementation to do that.

It could be the case that we need to actually make a bugfix in the homeserver rather than nio. However, if you can prove that the spec is saying (or get it changed via a clarification PR!) that the creator field is optional, then yes we can make that change in nio too.

PaarthShah avatar Jan 06 '24 18:01 PaarthShah

Hi @PaarthShah, thank you for the response. The home server setup follows well-known ansible book, Synapse with matrix-corporal to control some requirements on room creation.

In the prototyping, I use nio room_create to create the room. But the creator isn't passed to the server event as it should. Creating room manually via Cinny is working well. So I don't think it's an issue with the server setup.

So I believe that creator should be like you said, required. However, it would be better if it could be passed into the API call explicitly as a param.

I have some quick fixes to get rid of the validation message, hence it involves (1) and (2), however, it would be better if we could check how to improve it. In general, if there is a way to customize the log level, it would be fine as well. Otherwise, the bot that consumes the events will keep complaining.

xgenvn avatar Jan 06 '24 19:01 xgenvn

@xgenvn actually, turns out I was wrong and my above links are all pointing at the legacy spec:

The modern spec has already been updated by: https://github.com/matrix-org/matrix-spec-proposals/pull/2175

And as of room version 11, this has been implemented: https://spec.matrix.org/v1.9/changelog/

So the correct thing to do is indeed to make creator optional! Ideally, we should always read from the sender key regardless from the api response, instead, for simplicity.

You're welcome to make those/any other related changes

PaarthShah avatar Jan 06 '24 19:01 PaarthShah

@xgenvn Oh and btw nio currently uses the standard python logging module so any and all customizations that it supports with regards to filtering/setting the desired log level are already supported

PaarthShah avatar Jan 06 '24 19:01 PaarthShah

Hi @PaarthShah,

I agree on the sender key, that was my first thought as well, but you found the change to remove the creator which is great. So for minimal change, we can use option (1) for minimal change, is that okay? Or you want to remove completely the creator in the schema?

Thanks.

xgenvn avatar Jan 06 '24 19:01 xgenvn

@xgenvn Yep, option 1 for the minimal change, with the additional potential step of marking creator as a deprcated alias to sender.

PaarthShah avatar Jan 06 '24 19:01 PaarthShah

Sure, thanks @PaarthShah, I will try to make a PR in a day or so.

xgenvn avatar Jan 06 '24 19:01 xgenvn

@PaarthShah sorry for that long "a day or so". Please let me know if I need to update more on the PR. Thanks.

xgenvn avatar Apr 21 '24 15:04 xgenvn

@xgenvn Just approved runs 👌🏾 Will keep further comments specific to the PR there

PaarthShah avatar Apr 21 '24 16:04 PaarthShah