CKEditorError should use JSON.stringify carefully

[Reinmar]

I've been looking at a strange error:

    ✖ "before each" hook for "should be loaded"
      Chrome 59.0.3071 (Mac OS X 10.12.4)
    TypeError: Converting circular structure to JSON
        at JSON.stringify (<anonymous>)
        at CKEditorError (webpack:///packages/ckeditor5-utils/src/ckeditorerror.js:35:0 <- packages/ckeditor5-heading/tests/heading.js:108:26)
        at ParagraphCommand.decorate (webpack:///packages/ckeditor5-utils/src/observablemixin.js:319:0 <- packages/ckeditor5-heading/tests/heading.js:6711:10)

And the problem turned out to be that CKEditorError tries to do JSON.stringify() on the data object (the details of the error).

This fails because for the particular error which was thrown we log an instance of an object which has circ refs. If it was logged correctly I would realise what's wrong much faster, but this weird issue made me tracking a different bug.

---

If you'd like to see this feature implemented, add 👍 to this post.

[scofalik]

Isn't it a known issue that we "struggle" with all the time?

Honestly, to be really safe, we should take care that each class should be inspected to check whether it doesn't have a circular reference, and if so, define its own JSON.stringify. We don't do it regularly because we forget.

From time to time problem like this appears and we (or at least I :P) look where is the circular reference.

Anyway, AFAICS, this is a problem that I encountered a few times already and I thought that we all are aware of it.

[Reinmar]

I wasn't aware of it so far.

And the solution may be simpler – we iterate over the passed data's properties and write simple stringifcation for them because we need to handle the first level properties, but not further. If an object is encountered in one of those properties, it can be stringified to some [object ClassName] notation or something.

[wwwald]

This issue has come up a few times recently in the Trilium project - a note-taking Electron app that uses Ckeditor5 as the editing component. Example relevant issues are zadam/trilium#1427 and zadam/trilium#1490, showing the exact error message mentioned above about circular structures.

What's the status on this bug? Can we do something to move this forward?

[mlewand]

I saw it few times already. For instance today I got a websocketgateway.js:193 CKEditorError: cloud-services-internal-error: Invalid organization. Error Trace id: [redacted]. {"organizationId":"foo[redacted]","environmentId":"[redacted]"}

exception and after this there was a circular error:

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Socket'
    |     property 'io' -> object with constructor 'Manager'
    |     property 'nsps' -> object with constructor 'Object'
    --- property '/' closes the circle
    at JSON.stringify (<anonymous>)
    at new CKEditorError (ckeditorerror.js:62)
    at websocketgateway.js:195

I saw more of them past months (though not very often, that's for sure).

I think that rather expecting client code to avoid circular references we could make our CKEditorError handler a little smarter: use a custom replacer in JSON.stringify() that will strip the duplicated objects. That way will never have a crash due to circular deps and client code will not have to care about this rule.

[mmichaelis]

We observed just the very same issue trying to decorate a non-existing function (by accident... but nevertheless):

TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'InputTextView'
    |     property 'template' -> object with constructor 'Template'
    |     property 'attributes' -> object with constructor 'Object'
    |     ...
    |     index 3 -> object with constructor 'TemplateIfBinding'
    --- property 'observable' closes the circle
    at JSON.stringify (<anonymous>)
    at new CKEditorError (ckeditorerror.js?febc:62)
    at LinkFormView.decorate (observablemixin.js?f830:250)
[...]

I agree with @mlewand, that CKEditorError should handle any data at JSON.stringify(data):

ckeditor5/packages/ckeditor5-utils/src/ckeditorerror.js

Line 62 in be066b4

const message = `${ errorName }${ ( data ? ` ${ JSON.stringify( data ) }` : '' ) }${ getLinkToDocumentationMessage( errorName ) }`;

Possible reference: TypeError: cyclic object value - JavaScript | MDN.

[marcellofuschi]

Just created a PR that should fix the issue. It works as @mlewand suggested, i.e. CKEditorError now handles circular references without raising errors.