openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
546 stars 101 forks source link

Broken with cattrs >= 23.2.1 #430

Closed sedrubal closed 4 months ago

sedrubal commented 6 months ago

If I tried to upgrade the dependencies of my language server but for versions of cattrs >= 23.2.1, I get errors like this:

Error sending data
Traceback (most recent call last):
  File "myls/.venv/lib/python3.11/site-packages/pygls/protocol/json_rpc.py", line 386, in _send_data
    body = json.dumps(data, default=self._serialize_message)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
          ^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/json/encoder.py", line 258, in iterencode
    return _iterencode(o, 0)
           ^^^^^^^^^^^^^^^^^
  File "myls/.venv/lib/python3.11/site-packages/pygls/protocol/json_rpc.py", line 309, in _serialize_message
    return self._converter.unstructure(data)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "myls/.venv/lib/python3.11/site-packages/cattrs/converters.py", line 238, in unstructure
    return self._unstructure_func.dispatch(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<cattrs generated unstructure lsprotocol.types.TextDocumentPublishDiagnosticsNotification>", line 3, in unstructure_TextDocumentPublishDiagnosticsNotification
    'params': __c_unstr_params(instance.params),
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<cattrs generated unstructure lsprotocol.types.PublishDiagnosticsParams>", line 4, in unstructure_PublishDiagnosticsParams
    'diagnostics': __c_unstr_diagnostics(instance.diagnostics),
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "", line 2, in unstructure_iterable
  File "", line 2, in <genexpr>
  File "<cattrs generated unstructure lsprotocol.types.Diagnostic>", line 11, in unstructure_Diagnostic
    res['codeDescription'] = __c_unstr_code_description(instance.code_description)
                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "myls/.venv/lib/python3.11/site-packages/cattrs/converters.py", line 965, in unstructure_optional
    return None if val is None else _handler(val)
                                    ^^^^^^^^^^^^^
  File "<cattrs generated unstructure lsprotocol.types.CodeDescription>", line 3, in unstructure_CodeDescription
    'href': instance.href,
             ^^^^^^^^^^^^^
AttributeError: 'str' object has no attribute 'href'

Do you have some recommendations or ideas how to fix it?

alcarney commented 6 months ago

Can you provide a sample of the data causing the issue? From the error message it looks like there could be a CodeDescription that doesn't have the "shape" cattrs is expecting to see...

Also are you using exactly v23.2.1? That is a known "bad" version and was excluded in lsprotocol upstream

sedrubal commented 6 months ago

Thanks for the hint with the excluded version of cattrs. However, I also tried 23.2.2 and 23.2.4 (23.2.0 is yanked) and the problem is the same. The latest version that works is 23.1.2. I'm using python 3.11.

I think the code that causes the issue is this (as the traceback is somehow not complete, I'm not 100% sure):

@SERVER.feature(TEXT_DOCUMENT_DID_OPEN)
async def did_open(ls: LanguageServer, params: DidOpenTextDocumentParams):
    text_doc = ls.workspace.get_document(params.text_document.uri)
    # doing some analysis
    ...
    diagnostics: list[Diagnostic] = ...
    ls.publish_diagnostics(text_doc.uri, diagnostics)  # this might cause the crash
alcarney commented 6 months ago

I can reproduce your issue on cattrs==23.2.2 with the following code

>>> from lsprotocol import types
>>> from pygls.protocol import default_converter

>>> converter = default_converter()
>>> r = types.Range(start=types.Position(line=0,character=0), end=types.Position(line=0, character=0))
>>> d = types.Diagnostic(range=r, message="A diagnostic", code_description="a string")

>>> converter.unstructure(d)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/var/home/alex/Projects/pygls/.env/lib64/python3.12/site-packages/cattrs/converters.py", line 238, in unstructure
    return self._unstructure_func.dispatch(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<cattrs generated unstructure lsprotocol.types.Diagnostic>", line 11, in unstructure_Diagnostic
  File "/var/home/alex/Projects/pygls/.env/lib64/python3.12/site-packages/cattrs/converters.py", line 965, in unstructure_optional
    return None if val is None else _handler(val)
                                    ^^^^^^^^^^^^^
  File "<cattrs generated unstructure lsprotocol.types.CodeDescription>", line 3, in unstructure_CodeDescription
AttributeError: 'str' object has no attribute 'href'

And the same code works on cattrs=23.1.2

>>> from lsprotocol import types
>>> from pygls.protocol import default_converter

>>> converter = default_converter()
>>> r = types.Range(start=types.Position(line=0,character=0), end=types.Position(line=0, character=0))
>>> d = types.Diagnostic(range=r, message="A diagnostic", code_description="a string")

>>> converter.unstructure(d)
{'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 0}}, 'message': 'A diagnostic', 'codeDescription': 'a string'}

However, I get the feeling that a bugfix, released in 23.2.0 of cattrs has potentially revealed a bug in your language server that wasn't being caught before. If we look at the definition of a Diagnostic in the LSP Specification, we see that the code_description field should be a CodeDescription object rather than a plain string.

Tweaking the diagnostic definition to the following fixes the issue for me when using cattrs==23.2.2

>>> d = types.Diagnostic(range=r, message="A diagnostic", code_description=types.CodeDescription(href="a string"))
>>> converter.unstructure(d)
{'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 0}}, 'message': 'A diagnostic', 'codeDescription': {'href': 'a string'}}

Hope that helps!

alcarney commented 4 months ago

Closing as I think this is resolved now