wechaty / python-wechaty-puppet

Python Puppet Provider Abstraction for Wechaty
https://pypi.org/project/wechaty-puppet/
Apache License 2.0
12 stars 11 forks source link

EventHeartbeatPayload payload data structure error. #6

Closed zavierxu closed 4 years ago

zavierxu commented 4 years ago

When wechaty puppet recieved a heartbeat from hostie, an error occured: TypeError: __init__() got an unexpected keyword argument 'timeout'

https://github.com/wechaty/python-wechaty-puppet/blob/984e17390a78a406dc224b8538ee3d48b4ce9405/src/wechaty_puppet/schemas/event.py#L132-L134 Here class EventHeartbeatPayload() only defined one of the arguments data, left timeout behind, which may cause this error.

wj-Mcat commented 4 years ago

@zavierxu it's a greate issue about heartbeat event payload. This is translated from wechaty-puppet. This is a really data structure bug for python-wechaty-puppet, Thank your for finding it.

https://github.com/wechaty/wechaty-puppet/blob/ac9c87017234432d716fe55aa88874d22dd03412/src/schemas/event.ts#L80-L82

@huan I wonder if this is also a bug for wechaty-puppet ?

huan commented 4 years ago

What kind of bug so you think it is, could you please describe it in details?

wj-Mcat commented 4 years ago

python-wechaty-puppet-hostie receive EventHeartBeatPayload from hostie-server with two fields: data, timeout. I translate from puppet-wechaty, so EventHeartBeatPayload in python-wechaty have only one field: data. wechaty-puppet also have one field: data

I wonder if this is also a bug for wechaty-puppet ?

zavierxu commented 4 years ago

According to TypeScript handbook : https://www.typescriptlang.org/docs/handbook/interfaces.html#our-first-interface

Notice we didn’t have to explicitly say that the object we pass to printLabel implements this interface like we might have to in other languages. Here, it’s only the shape that matters. If the object we pass to the function meets the requirements listed, then it’s allowed.

So, it's valid in TypeScript, but not in Python

wj-Mcat commented 4 years ago

@zavierxu Thank you for reminding me. I have fixed this issue. Please reinstall wechaty-puppet pypi package later after this pr #7 being merged.

wj-Mcat commented 4 years ago

@huan although this is valid in grammar, but timeout field is actually leave out.

zavierxu commented 4 years ago

@zavierxu Thank you for reminding me. I have fixed this issue. Please reinstall wechaty-puppet pypi package later after this pr #7 being merged.

Thanks dude

huan commented 4 years ago

Thanks for the explanation and the fix.

It seems that the timeout property is unexpected data received from the server.

I believe we should always align with our golden truth from: wechaty-puppet. So the https://github.com/wechaty/wechaty-puppet/blob/ac9c87017234432d716fe55aa88874d22dd03412/src/schemas/event.ts#L80-L82, which you have used before should be the right data structure:

export interface EventHeartbeatPayload {
  data: string,
}

However, when we receive a payload that contains unexpected property, it seems that the Python dataclass will raise an exception?

Could we make the Python dataclass just ignore the unexpected property? If we can, I believe we can do that.

Or if it can't, then I agree with you that the #7 will be a good workaround.

At last, if we can find the reason that the puppet server sends the timeout, we can fix this problem at the server-side. a PR will be welcome if anyone can figure it out.

wj-Mcat commented 4 years ago

Ok,I will try to fix unexpected field exception at dataclass.

zavierxu commented 4 years ago

There are several workgrounds,for example this one, but it seems there is not an official way to do this.

huan commented 4 years ago

I have proposed a PR at https://github.com/wechaty/python-wechaty-puppet-hostie/pull/7, it's very concise and clean, I think it should be able to solve this problem?

zavierxu commented 4 years ago

It seems that this issue has been solved. I will close this issue.

huan commented 4 years ago

@wechaty/java @wechaty/go There are developers in our WeChat group talking about this issue, and so I mention you guys and hope we can fix it in languages other than Python as well.

See:

image