lektor / lektor

The lektor static file content management system
https://www.getlektor.com/
BSD 3-Clause "New" or "Revised" License
3.84k stars 309 forks source link

Nested flow lead to RecursionError #1168

Open nils-werner opened 1 year ago

nils-werner commented 1 year ago

When nesting flow blocks I'm starting to receive RecursionErrors in the admin screen which I didn't see before:

  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/datamodel.py", line 438, in <listcomp>
    x.to_json(pad, record, alt)
  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/datamodel.py", line 204, in to_json
    "type": self.type.to_json(pad, record, alt),
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/types/flow.py", line 228, in to_json
    rv["flowblocks"] = discover_relevant_flowblock_models(self, pad, record, alt)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/types/flow.py", line 27, in discover_relevant_flowblock_models
    return dict((k, v.to_json(pad, record, alt)) for k, v in all_blocks.items())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/types/flow.py", line 27, in <genexpr>
    return dict((k, v.to_json(pad, record, alt)) for k, v in all_blocks.items())
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/datamodel.py", line 437, in to_json
    "fields": [
              ^
  File "/home/nils/Projekte/wspz/.env/lib/python3.11/site-packages/lektor/datamodel.py", line 437, in <listcomp>
    "fields": [
              ^
RecursionError: maximum recursion depth exceeded

The versions I was using were

  1. lektor==3.0.1 (definitely working)
  2. lektor==3.1 (unsure)
  3. lektor==3.3.10 (definitely not working)

Unfortunately I'm unable to get the old versions to run right now, due to import errors in werkzeug but I'll continue trying to see if it's related to the version numbers.

nils-werner commented 1 year ago

Ah, I didn't define flow_blocks on some of my nested flowblocks. Once I define that, the error goes away... I'm now sure what changed the behaviour though.

dairiki commented 1 year ago

I think I've managed to reproduce this behavior

What Works

A set of recursive flowblocks as follows appears to work:

models/page.ini:

[model]
name = Page

[fields.blocks]
type = flow
flow_blocks = recurse

flowblocks/recurse.ini:

[block]
name = Recursive

[fields.flow_test]
type = flow
flow_blocks = text

flowblocks/text.ini:

[block]
name = Text

[fields.text]
type = markdown

Ways to Break Things

  1. If, the flow_blocks line in recurse.ini is removed, a RuntimeError("Nested flow-blocks require explicit list of involved blocks.") is (correctly) raised when attempting to edit a page in the admin GUI.

  2. If the flow_blocks lines are removed from both page.ini and recurse.ini, I see the recursion error that you describe.

  3. If, instead, the flow_blocks line in recurse.ini is modified to be self-referential, e.g.

    flow_blocks = text, recurse

    again, a (different) recursion error obtains.

Though I'm still not completely convinced that any of this behavior is new (if since 3.0.1 can be called "new" :wink:) the recursion errors in cases #2 and #3 could certainly be called bugs.

At this point, I think that there is no fundamental reason all three of the above cases couldn't be supported. That is, there's no fundamental we can't allow recursive flow fields to contain any flowblock type. The recursion loops in #2 and #3 occur while computing the list of flowblocks allowed in the recursive flow field. With correct caching/memoization I think the recursion loop could be avoided. (The check in #1 is there to prevent this same recursion loop.)