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

Empty eventData results in "Unparseable value: {'0': 0}" #134172

Open
01ste02 opened this issue Dec 28, 2024 · 4 comments
Open

Empty eventData results in "Unparseable value: {'0': 0}" #134172

01ste02 opened this issue Dec 28, 2024 · 4 comments

Comments

@01ste02
Copy link

01ste02 commented Dec 28, 2024

The problem

The connection to the zwave js server crashes upon receiving a notification with an empty eventData field due to parsing it as a dictionary.

What version of Home Assistant Core has the issue?

2024.12.5

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Supervised

Integration causing the issue

Z-Wave JS

Link to integration documentation on our website

https://www.home-assistant.io/integrations/zwave_js/

Diagnostics information

config_entry-zwave_js-01JFYDDACXK5C1E41E4M2E843W.json

Example YAML snippet

No response

Anything in the logs that might be useful for us?

2024-12-28 20:33:15.860 DEBUG (MainThread) [zwave_js_server.server] 2024-12-28T19:33:15.854Z:
2024-12-28T19:33:15.854Z SERIAL » [ACK]                                                                   (0x06)
2024-12-28 20:33:15.863 DEBUG (MainThread) [zwave_js_server.server] 2024-12-28T19:33:15.859Z:
2024-12-28T19:33:15.859Z DRIVER « [Node 003] [REQ] [ApplicationCommand]
                                  └─[Security2CCMessageEncapsulation]
                                    │ sequence number: 242
                                    │ security class:  S2_AccessControl
                                    └─[EntryControlCCNotification]
                                        sequence number: 5
                                        data type:       0
                                        event type:      3
                                        event data:      0x00
2024-12-28 20:33:15.871 INFO (MainThread) [zwave_js_server.server] 2024-12-28T19:33:15.863Z:
2024-12-28T19:33:15.863Z CNTRLR   [Node 003] [Notification] Entry Control
                                    event type: Disarm all
                                    data type:  None
2024-12-28 20:33:15.872 DEBUG (MainThread) [zwave_js_server] Received message:
WSMessage(type=<WSMsgType.TEXT: 1>, data='{"type":"event","event":{"source":"node","event":"notification","nodeId":3,"endpointIndex":0,"ccId":111,"args":{"eventType":3,"dataType":0,"eventData":{"0":0},"eventTypeLabel":"Disarm all","dataTypeLabel":"None"}}}', extra='')

2024-12-28 20:33:15.873 DEBUG (MainThread) [zwave_js_server] Listen completed. Cleaning up
2024-12-28 20:33:15.881 ERROR (MainThread) [homeassistant.components.zwave_js] Failed to listen: Unparseable value: {'0': 0}
2024-12-28 20:33:15.882 INFO (MainThread) [homeassistant.components.zwave_js] Disconnected from server. Reloading integration
2024-12-28 20:33:15.907 DEBUG (MainThread) [zwave_js_server] Trying to connect
2024-12-28 20:33:15.918 DEBUG (MainThread) [zwave_js_server] Received message:
WSMessage(type=<WSMsgType.TEXT: 1>, data='{"type":"version","homeId":3234819789,"driverVersion":"14.3.7","serverVersion":"1.40.2","minSchemaVersion":0,"maxSchemaVersion":40}', extra='')

Additional information

The device is a philio tech PSK01 (which I know you can read from the diagnostics etc). Upon entering the correct user code and pressing "Disarm", the device seemingly sends three notifications which are the following:

12/28/2024, 9:31:06 PM - notification
Arg 0:
113
Arg 1:
└─type: 6
└─event: 6
└─label: Access Control
└─eventLabel: Keypad unlock operation
└─parameters
└──userId: 1
12/28/2024, 9:31:06 PM - value updated
Arg 0:
└─commandClassName: Notification
└─commandClass: 113
└─property: alarmLevel
└─endpoint: 0
└─newValue: 0
└─prevValue: 0
└─propertyName: alarmLevel
12/28/2024, 9:31:06 PM - value updated
Arg 0:
└─commandClassName: Notification
└─commandClass: 113
└─property: alarmType
└─endpoint: 0
└─newValue: 0
└─prevValue: 0
└─propertyName: alarmType
12/28/2024, 9:31:05 PM - notification
Arg 0:
111
Arg 1:
└─eventType: 3
└─dataType: 0
└─eventData
└─eventTypeLabel: Disarm all
└─dataTypeLabel: None

Only the one with the empty eventData seems to be causing this issue.

If I interpret things correctly, https://github.com/home-assistant-libs/zwave-js-server-python is used for communicating with the addon. The issue lies there, and seems to be from the following trace:

client.py->listen()->receive_until_closed()->_handle_incoming_message()
then row 533, self.driver.receive_event(event)
which leads to line 99 in driver.py:
DRIVER_EVENT_MODEL_MAP[event.type].from_dict(event.data)
which due to the eventType field of the faulty notification leads to a creation of an
EntryControlNotification (defined in notification.py, called via from_dict in event.py)

Row 81/82 in notification.py looks as follows:

if event_data := self.data["args"].get("eventData"):
            self.event_data = parse_buffer(event_data)

where parse_buffer is defined in helpers.py, which calls parse_buffer_from_dict. In this file on row 42, the error is thrown.

I believe the issue to be triggered by the JSON parsing in client.py, _receive_json_or_raise(). This interprets the empty eventData from the addon, and somehow expects a dict and interprets None/empty as {'0': 0}. If the parsing there is expected to behave this way, I believe that row 81 in notification.py needs to check if self.data["args"].get("eventData") is not {'0': 0} before calling parse_buffer.

Since the issue seems to be upstream from this project, I presume that I should open this as an issue there as well?
(Their issue template states: "Please report issues with Z-Wave JS in the Home Assistant core repository unless a developer told you otherwise. ", hence the post here. Plus that possible fixes will eventually have to be integrated here.)

Thanks in advance!

@home-assistant
Copy link

Hey there @home-assistant/z-wave, mind taking a look at this issue as it has been labeled with an integration (zwave_js) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of zwave_js can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign zwave_js Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


zwave_js documentation
zwave_js source
(message by IssueLinks)

@kpine
Copy link
Contributor

kpine commented Dec 29, 2024

Looks like there may be two issues here.

The first is that there's something odd with how your device is reporting that notification, and I'm not sure it's valid behavior. I would recommend also reporting the issue to https://github.com/zwave-js/node-zwave-js as they might want to implement some fix to handle this possibly invalid case. This is the report from the device:

2024-12-28T19:33:15.859Z DRIVER « [Node 003] [REQ] [ApplicationCommand]
                                  └─[Security2CCMessageEncapsulation]
                                    │ sequence number: 242
                                    │ security class:  S2_AccessControl
                                    └─[EntryControlCCNotification]
                                        sequence number: 5
                                        data type:       0
                                        event type:      3
                                        event data:      0x00

It reports data type 0, which is NA and means "No data included". Yet it also reports a single 0-byte of event data (evidenced by the hex string 0x00), so it's not actually "empty". I'm not sure that's valid by the Z-Wave spec, as the existence of the data conflicts with the data type. According to the Z-Wave specification, "The format of this field [event data] MUST comply with the data format advertised by the Data Type field." In my interpretation, complying with the "no data included" type would mean there's no data included... but I would leave that for node-zwave-js to answer.

The second problem is looking like a result of a breaking change in Z-Wave JS v14 that isn't accounted for by the zwave-js-server. In the v14 migration guide section "Replaced Node.js Buffer with Uint8Array or portable Bytes class":

the use of Buffers was replaced with Uint8Arrays where applicable.
...
In output positions however, this is a breaking change.
...
Both Uint8Array and Bytes can easily be converted to a Buffer instance if absolutely necessary by using Buffer.from(...).

The eventData field of the relevant notification is now a Bytes type when it used to be Buffer. The zwave-js-server converts all outgoing messages to JSON using Node JS's JSON.stringify function, so this qualifies as an "output position", and there are different results:

> const data = new Uint8Array([0])
undefined
> JSON.stringify(data)
'{"0":0}'
> JSON.stringify(Buffer.from(data))
'{"type":"Buffer","data":[0]}'
>

Indeed it's breaking because you can see this 0-byte array reported by the device is no longer encoded as a stringified Buffer as the Python library is expecting, so it asserts in the code you identified. This data was incoming from the server as shown in the websocket log message:

WSMessage(type=<WSMsgType.TEXT: 1>, data='{"type":"event","event":{"source":"node","event":"notification","nodeId":3,"endpointIndex":0,"ccId":111,"args":{"eventType":3,"dataType":0,"eventData":{"0":0},"eventTypeLabel":"Disarm all","dataTypeLabel":"None"}}}', extra='')

So the zwave-js-server needs to properly convert Bytes/Uint8Array types to Buffer during message serialization for outgoing messages. I would also argue that the Python library should not assert and crash the integration because of such an error.

The same notification event in driver v13.10.3 should work fine, so if you have the option downgrading that would workaround this.

@01ste02
Copy link
Author

01ste02 commented Dec 29, 2024

So just to double check, you believe that I should open this as an issue in both https://github.com/home-assistant-libs/zwave-js-server-python and https://github.com/zwave-js/node-zwave-js as the first should get error handling for these cases, and the latter should fix the stringification of the JSON objects?

@kpine
Copy link
Contributor

kpine commented Dec 29, 2024

No, no, no...

I'm suggesting you submit a bug to node-zwave-js to ask whether event data should be returned for the NA data type. Not sure if that behavior is expected or not, they may be fine with the current behavior, I think it's weird. Submitting the issue is up to you. node-zwave-js has nothing to do with the conversion of the event data to JSON, that's zwave-js-server.

Otherwise, this issue is fine.

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

No branches or pull requests

2 participants