Version 4 cyclic refs freezes without any error

[AndyOGo]

What did you do

I just upgraded to version 4 from verison 3 and tried to generate md files by running my NPM docs script:

npm run docs
# which executs: jsonschema2md -o docs/ -x docs/ -d schemas/ -e json

What did you expect to happen

All markdown files to be generated

What happened

The CLI programm freezes without any error.
Last log is:

loading schemas

What's your environment

• Operating System: macOS Catalina 10.15.2 (19C57)
• node.js version: v10.15.0

Do you have example files:

https://github.com/axa-ch/cyclic-schemas-test

[trieloff]

Hi Andy, the external refs shouldn't be the problem, as jsonschema2md will try to resolve them, but simply ignore them if they can't be resolved. It's hard to troubleshoot without the schema files, so some things that might establish the direction:

1. how many schema files do you have?
2. are there recursive references?

[AndyOGo]

Hi @trieloff , thank you for your quick answer.
Great, that they are ignored if they can't be resolved.

1. we have 57 schemas locally on disk
2. yes some are used recursively and we have intended cyclic references

We use our schemas to describe a generic component-tree, which may have arbitrarily nested components.

[trieloff]

Could you try to create a minimal example, just to validate the assumption that it has something to do with the recursion? e.g. three schemas that form a cycle.

[AndyOGo]

@trieloff here you go :)
https://github.com/axa-ch/cyclic-schemas-test

Please let me know if you need any more data

[trieloff]

@AndyOGo the next release should have a fix. At least a fix that works with the example you've shared.

[AndyOGo]

@trieloff sounds great, thank you very much for your quick handling.
I'm looking forward to try it with next release :)

[trieloff]

🎉 This issue has been resolved in version 4.0.1 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[AndyOGo]

@trieloff Thanks for the release.
I upgraded to 4.0.2, though I still end up in an infinite loop 🤔

We heavily rely on recursion in array items mostly with allOf or oneOf descriptors.

[trieloff]

I'll have a look, but it might not get done in this decade 😉 expect a fix in the early 2020s.

[tripodsan]

still happens for me, too.

[tripodsan]

@trieloff, the problem happens, like @AndyOGo said, with a recursive ref.

for example:

  "$id": "https://ns.adobe.com/helix/shared/conditions",
   ...
   "properties": {
        "and": {
            "type": "array",
            "items": {
                "$ref": "https://ns.adobe.com/helix/shared/conditions"
            },
            "description": "All conditions in this list must be met"
        },
...

By looking at the code, I don't now how this should work, as the proxy resolves the $ref my mixing in the referenced schema. and then the JSON writer recursively goes down the tree...

it might be possible to try to detect a circular reference, and only avoid the mixin in this case, but I think, also for readability of the MD, having a link to the referenced schema is good enough.

alternatively, we could just not write the schemas JSON, and ensure that the markdown renders the $ref correctly.

[AndyOGo]

hmh, I think a lazy cache should handle the complexity 🤔

1. if $ref is not in cache, then traverse it and cache it's result (I guess link to md file would be enough)
   a. Make sure to block traversion of self-referencing schemas
2. if $ref is in cache, skip traversion and just reuse existing result

[tripodsan]

the is not the problem. the $ref is in the cache. but the schema itself is infinitely recursive. so when writing the json of the schema back...what should be written? I think resolving the $ref in the schema is wrong, as there is no good way of representing the expanded tree anyways.

[AndyOGo]

Hmh, maybe I miss some info.
But I think stopping traversion if $ref is a cyclic dependency should handle the infinite loop, not?

[tripodsan]

look at the example above...the condition has a and property which needs to be condition.
even if there would be an intermediate step, what would be the point of expanding it? it would only confuse the documentation.

maybe you have a different example of a $ref that you like to be expanded in the documentation...

also note: the error only occurs when writing the json again (@trieloff what is the point of this, actually?). and not when writing the markdown.

[AndyOGo]

Thanks for the hint.

At which point is the cache hot?
Because if cache is only set upon completion of traversion it wont work with schemas which reference itself. Because there will never be a cache set!

[trieloff]

also note: the error only occurs when writing the json again (@trieloff what is the point of this, actually?). and not when writing the markdown.

Ah, then the fix should be relatively easy. We write the JSON again so that my.schema.json my.description.md and my.1.example.json get merged into one document. The separate documents are easier to author, but the merged document is easier to consume by other tooling.

@AndyOGo a quick workaround for you might be to disable the JSON output.

@tripodsan I think our error is that the writeJsonSync function tries to serialize the proxied JSON Schema, which is recursive and gets caught up in a loop. We can circumvent this by merging the description and examples into the original (non-proxied and non-recursive) JSON and then just writing this.

---

The markdown processing doesn't get caught in an endless loop, because we generate one output for each schema, and the schema identity is independent from the depth in the recursive tree.

[tripodsan]

We can circumvent this by merging the description and examples

but you would only do this on the toplevel, right? so wouldn't it be enough to disable the $ref resolution?

jsonschema2md/lib/schemaProxy.js

Lines 104 to 115 in d6a7cb9

	if (retval[keyword`$ref`]) {
	const [uri, pointer] = retval.$ref.split('#');
	// console.log('resolving ref', uri, pointer, 'from', receiver[symbols.filename]);
	const basedoc = uri || receiver[symbols.id];
	if (schemas.known[basedoc]) {
	const referenced = schemas.known[basedoc][symbols.resolve](pointer);
	// inject the referenced schema into the current schema
	Object.assign(retval, referenced);
	} else {
	console.error('cannot resolve', basedoc);
	}
	}

[trieloff]

Yes, the merging only happens on the top level, but I can take the issue from here.

[tripodsan]

but I can take the issue from here.

ok. thanks.

[trieloff]

🎉 This issue has been resolved in version 4.0.3 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[AndyOGo]

Thanks for your help.
But sorry, I still end up in an infinite loop with version 4.0.3