Fix JSON.stringify issue with circular ref objects

[TessavWalstijn]

Description

Fixes #15291

Type of Change

• Bugfix
• Improvement
• New Feature
• Refactor / codestyle updates
• Other, please describe:

Requirements Checklist

• New / updated tests are included
• All tests are passing locally
• Performed a self-review of the submitted code

If adding a new feature:

• Documentation was added/updated. PR:

Test note

I did look at adding tests for create and update with an circular ref object.
However since there where not object tests already, it felt out of scope for this issue.

[azrikahar]

Thanks for the PR! Although the error does technically happens in payload service, should we add this fix in Run Script operation (or flows in general) instead?

In practice I presume fixing this in payload service does have merit as it also cover cases such as custom extensions, but that does beg the question of whether it should be something handled by the developer, provided there's not really a straight forward way to hit this issue without "dev-centric" avenues such as Run Script operation and custom extensions. Plus this doesn't seem to be reported prior from the Run Script scenario, so perhaps we can opt for fixing it on the Run Script level for now.

As for the util itself, do we want to use a package such as https://www.npmjs.com/package/fast-safe-stringify? 🤔 At a glance, I assume a slight downside is it'll only show [Circular] rather than [Circular]a.b.c, but the depth and edge limit does seem nifty as well. I'm also unsure about the licensing perspective.

Or alternatively, @licitdev did also bring up a solution of forgoing any util or package for stringify with circular ref, and add a tryJSON which throws the error there so that the operation itself can go down the reject path of the flow! @rijkvanzanten thoughts on this particular alternative?

[br41nslug]

As for the util itself, do we want to use a package such as https://www.npmjs.com/package/fast-safe-stringify?

I would be in favor of using a package over copy-pasting code from stack overflow

[TessavWalstijn]

As for the util itself, do we want to use a package such as https://www.npmjs.com/package/fast-safe-stringify? 🤔 At a glance, I assume a slight downside is it'll only show [Circular] rather than [Circular]a.b.c, but the depth and edge limit does seem nifty as well

I have chosen this one from stack overflow with the reason it has the depth tree after the [Circular]a.b.c.

I haven't found it like that in other packages. Depending on what the business value is of a circular object I do understand for going the way of a package.

---

I'm also unsure about the licensing perspective.

Also Licensing should not be a problem since Directus is under a GPL-3.0 License and the CC BY-SA 4.0 License is one way compatible to GPL-3.0 License to read more

• Creative Commons
• Free Software Foundation

---

An other way of solving this could be testing the custom code part if it has a circular reference and trow an error at that location in the UI that it is an illegal return object.

[azrikahar]

I have chosen this one from stack overflow with the reason it has the depth tree after the [Circular]a.b.c.

I haven't found it like that in other packages. Depending on what the business value is of a circular object I do understand for going the way of a package.

Makes sense! That would indeed be a great information to have, but it's also a good question of what the value is as mentioned. A significant one would probably be a better error message since it'll be more obvious what are the exact circular refs.

Also Licensing should not be a problem since Directus is under a GPL-3.0 License and the CC BY-SA 4.0 License is one way compatible to GPL-3.0 License to read more

• Creative Commons
• Free Software Foundation

TIL!

An other way of solving this could be testing the custom code part if it has a circular reference and trow an error at that location in the UI that it is an illegal return object.

This seems to align with the alternative approach as well. Quote from previous comment:

Or alternatively, @licitdev did also bring up a solution of forgoing any util or package for stringify with circular ref, and add a tryJSON which throws the error there so that the operation itself can go down the reject path of the flow!

It does seem to be a good solution to explore, especially in the context of Flows.

[rijkvanzanten]

[...] and add a tryJSON which throws the error there so that the operation itself can go down the reject path of the flow!

This makes a ton of sense to me. Instead of trying to auto-fix it in a way that may or may not be expected, just crash gracefully and let the user resolve!

[connorwinston]

This PR is being closed as the behavior is being fixed at the flows level in #17519 as Rijk mentioned above. We appreciate the time you took to help us get to the bottom of this :)