akarneliuk / pygnmi

The pure Python implementation of the gNMI client.
https://training.karneliuk.com
BSD 3-Clause "New" or "Revised" License
129 stars 44 forks source link

_merge_updates() does not handle case where new_resp is None #102

Open ipmonk opened 2 years ago

ipmonk commented 2 years ago

Crash seen when connecting to a Juniper box running Junos: 21.4R2-S1.12-EVO

# Modules
from pygnmi.client import gNMIclient, telemetryParser

# Variables
host = ('1.1.1.1', '50051')

# Body
subscribe = {
    'subscription': [
        {
            'path': '/components/component',
            'mode': 'sample',
            'sample_interval': 10000000000
        },
    ],
    'use_aliases': False,
    'mode': 'stream',
    'encoding': 'json'
}

if __name__ == '__main__':
    with gNMIclient(target=host, username='foo', password='lab123', insecure=True) as gc:
        telemetry_stream = gc.subscribe_stream(subscribe=subscribe)
        for telemetry_entry in telemetry_stream:
            print(telemetry_entry)

resulted in an unhandled exception:

Parsing of telemetry information is failed.
Traceback (most recent call last):
  File "/Users/foo/scripts/foo/gnmi-scripts/subscribe.py", line 25, in <module>
Exception in thread Thread-5:
Traceback (most recent call last):
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/threading.py", line 973, in _bootstrap_inner
    for telemetry_entry in telemetry_stream:
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 973, in __next__
    self.run()
  File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/threading.py", line 910, in run
    result = self.next()
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 985, in next
    self._target(*self._args, **self._kwargs)
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 908, in enqueue_updates
    return self._next_update(timeout=None)
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 1036, in _next_update
    for update in subscription:
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/grpc/_channel.py", line 426, in __next__
    return self._get_updates_till_sync(timeout=timeout)
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 949, in _get_updates_till_sync
    return self._next()
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/grpc/_channel.py", line 809, in _next
    self._merge_updates(resp, new_resp)
  File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 954, in _merge_updates
    if 'update' in new_resp:
TypeError: argument of type 'NoneType' is not iterable
    raise self
grpc._channel._MultiThreadedRendezvous: <_MultiThreadedRendezvous of RPC that terminated with:
    status = StatusCode.CANCELLED
    details = "Channel closed!"
    debug_error_string = "UNKNOWN:Error received from peer 1.1.1.1:50051 {grpc_message:"Channel closed!", grpc_status:1, created_time:"2022-10-25T18:22:42.347026+11:00"}"

workaround was to add a line to client.py in _merge_updates()

    def _merge_updates(self, resp, new_resp):
        if new_resp is not None:         ### adding this line
            if 'update' in new_resp:
                for key in new_resp['update']:
                    if key.upper() in UpdateResult.Operation.keys():
                        if key not in resp['update']:
                            resp['update'][key] = []
                        resp['update'][key] += new_resp['update'][key]
                    else:
                        resp['update'][key] = new_resp['update'][key]
            if 'sync_response' in new_resp:
                resp['sync_response'] = new_resp['sync_response']
ipmonk commented 2 years ago

With the workaround in place, there appears to be multiple packets which fail to parse correctly, before it returns data:

% python3 subscribe.py Parsing of telemetry information is failed. Parsing of telemetry information is failed. Parsing of telemetry information is failed. \<snip> Parsing of telemetry information is failed. Parsing of telemetry information is failed. {'update': {'update': [{'path': 'value', 'val': 33572}, {'path': 'value', 'val': 34081}, {'path': 'value', 'val': 0}, {'path': 'value', 'val': 0}, {'path': 'value', 'val': 0}, {'path': 'value', 'val': 0}, {'path': 'value', 'val': 0}, {'path': 'value', 'val': 0}, {'path': 'value',...

akarneliuk commented 2 years ago

hey @ipmonk Thanks for flagging. It is quite curios, what Juniper sends that it is not parsable.. Did you talk to them about the issue? I'd agree that workaround may be a good way to go. Thanks Anton

ipmonk commented 2 years ago

Hi Anton,

debugged this further, it is getting tripped up by this path:

  update {
    path {
      elem {
        name: "state"
      }
      elem {
        name: "temperature"
      }
      elem {
        name: "instant"
      }
    }
    val {
      json_val: ""
    }
  }

'Traceback (most recent call last):\n File "/Users/foo/scripts/foo/gnmi-scripts/venv/lib/python3.9/site-packages/pygnmi/client.py", line 1135, in telemetryParser\n update_container.update({'val': json.loads(update_msg.val.json_val)})\n File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/json/init.py", line 346, in loads\n return _default_decoder.decode(s)\n File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/json/decoder.py", line 337, in decode\n obj, end = self.raw_decode(s, idx=_w(s, 0).end())\n File "/Library/Developer/CommandLineTools/Library/Frameworks/Python3.framework/Versions/3.9/lib/python3.9/json/decoder.py", line 355, in raw_decode\n raise JSONDecodeError("Expecting value", s, err.value) from None\njson.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n File "/Users/foo/.vscode/extensions/ms-python.python-2022.18.2/pythonFiles/lib/python/debugpy/_vendored/pydevd/_pydevd_bundle/pydevd_resolver.py", line 192, in _get_py_dictionary\n attr = getattr(var, name)\nAttributeError\n'

However, If I change the encoding to PROTO, we get this data, and no crash is seen.

update {
    path {
      elem {
        name: "state"
      }
      elem {
        name: "temperature"
      }
      elem {
        name: "instant"
      }
   }
    val {
      float_val: 47        <<<<<<
    }
  }

According to JNPR, PROTO is the recommended encoding when using Subscribe() That said, they are looking into why JSON encoding isn't returning a value. IMO, the error checking in pygnmi should be improved to handle cases where unexpected values are returned. Alternatively, you could enforce PROTO as the only supported encoding for Subscribe()

ipmonk commented 2 years ago

Another example of json_val which causes an exception:

  update {
    path {
      elem {
        name: "state"
      }
      elem {
        name: "temperature"
      }
      elem {
        name: "instant"
      }
    }
    val {
      json_val: "\360\341B\355\375\177"
    }
  }