Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when starting with matrix-connector #1159

Closed
awesome-michael opened this issue Oct 13, 2019 · 22 comments · Fixed by #1166
Closed

Error when starting with matrix-connector #1159

awesome-michael opened this issue Oct 13, 2019 · 22 comments · Fixed by #1166
Labels

Comments

@awesome-michael
Copy link
Contributor

When I try to start opsdroid I keep getting the following error. I double checked indentation and when I delete the matrix-connector part opsdroid starts fine.

opsdroid_1     | Error validating data /root/.config/opsdroid/configuration.yaml with schema /usr/src/app/opsdroid/configuration/schema.yaml
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.webhook-url: Required field missing
opsdroid_1     | 	connectors.1.access-token: Required field missing
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.verify-token: Required field missing
opsdroid_1     | 	connectors.1.page-access-token: Required field missing
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.mxid: 'xxx' is not a regex match.
opsdroid_1     | 	connectors.1.room: Required field missing
opsdroid_1     | 	connectors.1.rooms.main: 'xxx' is not a regex match.
opsdroid_1     | 	connectors.1.rooms.other: 'xxx' is not a regex match.
opsdroid_1     | 	connectors.1.homeserver: 'xxx' is not a regex match.
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.api-token: Required field missing
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.token: Required field missing
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.consumer_key: Required field missing
opsdroid_1     | 	connectors.1.consumer_secret: Required field missing
opsdroid_1     | 	connectors.1.oauth_token: Required field missing
opsdroid_1     | 	connectors.1.oauth_token_secret: Required field missing
opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.
opsdroid_1     | 	connectors.1.room-id: Required field missing
opsdroid_1     | 	connectors.1.access-token: Required field missing

Configuration File

##                      _           _     _
##   ___  _ __  ___  __| |_ __ ___ (_) __| |
##  / _ \| '_ \/ __|/ _` | '__/ _ \| |/ _` |
## | (_) | |_) \__ \ (_| | | | (_) | | (_| |
##  \___/| .__/|___/\__,_|_|  \___/|_|\__,_|
##       |_|
##                   __ _
##   ___ ___  _ __  / _(_) __ _
##  / __/ _ \| '_ \| |_| |/ _` |
## | (_| (_) | | | |  _| | (_| |
##  \___\___/|_| |_|_| |_|\__, |
##                        |___/
##
## A default config file to use with opsdroid
## see https://github.com/opsdroid/opsdroid/blob/master/opsdroid/configuration/example_configuration.yaml for more config options

## Set the logging level
logging:
  level: info
  path: opsdroid.log
  console: true

## Show welcome message
welcome-message: true

## Connector modules
connectors:

  - name: websocket
    # optional
    bot-name: "mybot" # default "opsdroid"
    max-connections: 10 # default is 10 users can be connected at once
    connection-timeout: 10 # default 10 seconds before requested socket times out

  ## Matrix (core)
  - name: matrix
    # Required
    mxid: "xxx"
    password: "xxx"
    # a dictionary of multiple rooms
    # One of these should be named 'main'
    rooms:
      'main': 'xxx'
      'other': 'xxx'
    # Optional
    homeserver: "xxx"
    nick: "xxx"  # The nick will be set on startup
    room_specific_nicks: False  # Look up room specific nicknames of senders (expensive in large rooms)

## Skill modules
skills:

  ## Dance (https://github.com/opsdroid/skill-dance)
  - name: dance

  ## Hello (https://github.com/opsdroid/skill-hello)
  - name: hello

  ## Loudnoises (https://github.com/opsdroid/skill-loudnoises)
  - name: loudnoises

  ## Seen (https://github.com/opsdroid/skill-seen)
  - name: seen
@jacobtomlinson
Copy link
Member

cc @Cadair @SolarDrew

@Cadair
Copy link
Contributor

Cadair commented Oct 14, 2019

The config looks fine to me, I am little confused as to how this:

opsdroid_1     | 	connectors.1.name: 'matrix' is not a regex match.

is being triggered off this regex?

@awesome-michael
Copy link
Contributor Author

I forgot to mention that I'm using the docker image opsdroid/opsdroid:latest pulled yesterday. So should be v0.16.0

@jacobtomlinson
Copy link
Member

@Cadair yeah it's that regex. I've seen something similar when indentation has been off. There aren't any other options missing in the config are there?

@FabioRosado
Copy link
Member

FabioRosado commented Oct 14, 2019

Could this be some issue with the yamale check? I remember the issue I had with Telegram(#1055) the messages seems somewhat similar to what I was getting 🤔

--EDIT--
With the yamale checks in place, I'm going to assume that they are being thrown because of the xxx that is being passed. Perhaps one of the options is marked as required and doesn't accept anything that is not in the regex check?

--EDIT 2--

I check the schema.yaml and it seems that my suspicion was correct, the issue was some of the options that was passed to the configuration file. You can see on line 71 and line 75 that the regex expects the string to end with :matrix.org or the homeserver to be https://matrix.org.

I believe the regex for rooms-main should be changed as well otherwise it will always fail if someone self-hosts matrix.

@awesome-michael were you running matrix on your own server? If that's the case then I am going to say that I'm 100% sure the issue is here 🤔

@awesome-michael
Copy link
Contributor Author

Yes I'm trying to connect to my own homeserver. In fact I'm trying to connect to pantalaimon that should connect to my own homeserver as suggested in #992 ;)

I'll check if editing the regex will help with this issue.

@FabioRosado
Copy link
Member

Please let us know but I think it will be one of these fields (if not all of then) that are causing the issues. For what I've gathered they assume you are using matrix.org only.

Please feel free to raise a PR fixing the issue if you wish otherwise i'll try to tackle it when I got the time once it has been confirmed 😄 👍

@Cadair
Copy link
Contributor

Cadair commented Oct 14, 2019

the regex expects the string to end with :matrix.org or the homeserver to be https://matrix.org.

🙈 Why?! 😱

@Cadair
Copy link
Contributor

Cadair commented Oct 14, 2019

Looks like this has been busted since the introduction of these schemas: #1003

I think we probably shouldn't be validating any of those fields anywhere near as strongly as that, I am nervous about validating them at all beyond type really.

@jacobtomlinson
Copy link
Member

Ok let's roll the validation back a bit.

I'm also not very impressed with the error handling in yamale. It seems to just blast a ton of errors at you

@FabioRosado
Copy link
Member

FabioRosado commented Oct 14, 2019

the regex expects the string to end with :matrix.org or the homeserver to be https://matrix.org.

🙈 Why?! 😱

Perhaps the contributor didn't knew the matrix connector could connect to own servers 🤔

Instead of reverting the validation PR how about we fix the schema? I'm with Cadair about validating the yaml beyond type - it makes sense on the required things but doesn't on other situations like this one.

Yamale allows you to just specify type like this:

friend:
    name: str()

We could check if some things are strings, lists, dicts, etc. You can also write custom validators but I'm not sure if it will be used much atm.

I also agree that the error handling in yamale is pretty bad since it just regurgitates the errors at you and you need to cross check with the schema what is expected. (unless this is an issue with the regex validator)

@jacobtomlinson
Copy link
Member

Instead of reverting the validation PR how about we fix the schema?

Yeah I didn't mean revert the PR. I meant relax the rules.

@FabioRosado
Copy link
Member

Oh sorry misunderstood the comment!

I did a quick look at yaml validators and Cerberus seems like a good way to get error handling 🤔

What do you think? Cerberus Usage

@jacobtomlinson
Copy link
Member

Would that be a replacement for yamale?

@FabioRosado
Copy link
Member

If we want better error handling we might have to replace it yeah, not sure how it compares with yamale yet though

@awesome-michael
Copy link
Contributor Author

awesome-michael commented Oct 14, 2019

okay I verified that the regex checks are responsible for the error. I changed the scheme.yaml to

...
---                                      
matrix:                                  
  name: regex('^(?i)matrix$')            
  mxid: str(required=True)               
  password: str(required=True)           
  room: str()                      
  rooms: (include('room1'))        
  homeserver: str(required=True)   
  nick: str(required=False)        
  room_specific_nicks: bool(required=False)
  send_m_notice: bool(required=False)      
---                                        
room1:                                     
  'main': str()                            
  'other': str()                           
---  
...

(the rooms were checked to end with matrix.org too)

and altered my config to

 ## Matrix (core)
  - name: matrix
    # Required
    mxid: "xxx"
    password: "xxx"   
    # a dictionary of multiple rooms
    # One of these should be named 'main'
    rooms:
      'main': 'xxx'
      'other': 'xxx'
   # Optional
    homeserver: "xxx"
    nick: "xxx"  # The nick will be set on startup
    room_specific_nicks: False  # Look up room specific nicknames of senders (expensive in large rooms)
    webhook-url: ''
    access-token: ''
    verify-token: ''
    page-access-token: ''
    room: ''
    api-token: ''
    token: ''
    consumer_key: ''
    consumer_secret: ''
    oauth_token: ''
    oauth_token_secret: ''
    room-id: ''
    access-token: ''

I had to provide all the Required field missing from the error above too.
So it seems the schema check for Matrix is really broken :(
As I read through the discussion I think I won't be able to provide a PR for this.

@FabioRosado
Copy link
Member

FabioRosado commented Oct 14, 2019

If you are participating on the hacktoberfest you can just change the schema for what you have show shown on your comment that seems a good temporary fix until we decide what to do next.

If not or you don’t have time I will open a PR to fix the issue tomorrow 😄👍 anyway thank you for raising the issue and helping us with this!

@awesome-michael
Copy link
Contributor Author

Since hacktoberfest seems to be perfect occasion for this (would be my first PR) I begun to write a fix. Now I think I have an explanation for all the Required field missing errors:
all these fields are checked with str() in the other sections. Looks like str() is equivalent to str(required=True) and the parser is ignoring that these fields come from other sections.
How to deal with that problem?

@FabioRosado
Copy link
Member

That’s a good question, I don’t understand why yamale decided to just go add the required if you specify a type, this is pretty weird...

It’s a pain in the back but perhaps turn all of those that are not required to False?

@awesome-michael
Copy link
Contributor Author

Was my first thought too.. but I think these values are checked with str() because they are all required for their sections...

@awesome-michael
Copy link
Contributor Author

I read a bit through the documentation of yamale..
It says

Schema files may contain more than one YAML document (nodes separated by ---)

and

Every root node not in the first YAML document will be treated like an include

so the schema.yaml you built can not check for the individual connectors but checks always everything. Thats the reason it complains about the required fields so much.

@awesome-michael
Copy link
Contributor Author

Okay.. yamale seems not to see all the fields as required, but when an error occurs all these fields are shown as "missing required fields". If the config is fine the fields aren't reported. So it's "just" the error handling that is misleading.

I opened a PR to address the actual issue: #1166
I was unsure if the mxid and room addresses should rather be checked with a regex to assure they are valid or if that would be unnecessary complex..

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

Successfully merging a pull request may close this issue.

4 participants