Asserting and debugging mechanisms

[Reinmar]

We should have mechanisms in the dev code to:

1. Create assertions in the dev code. For example - we may want to validate in some critical method whether an argument is correct or at some stage of a long and complex algorithm whether an intermediate result is ok (that not always can be done by tests).
2. Log some message with a chosen verbosity level. This, thanks to multiple verbosity levels, can be used either to notify the developer about some important problems (like that an editor is already initialised on the given element) or to debug (by enabling the verbose mode). Having logging in places which we consider critical for debugging many algorithms like when the filler char is added and removed or the selection is locked or set or editable focused or blurred would make it possible to simplify debugging by enabling verbose logging.

Additional requirement is that it would be good to keep these instructions in the build mode, but at the same time we don't want to affect the code size. The idea was that we can use error codes which in the dev version will be automatically replaced by messages (loaded from separate files) and in the build mode the developer will be notified where to find the message.

Note that by logging most important events it will be easier for us to understand reported issues. The logged messages can be automatically stored and then we'll ask the developer to paste CKEDITOR.log.dump() result.

[fredck]

Uglify2 (the program we use for minification), accepts "global definitions", which can be used in the code. For example, we could have the DEBUG constant that will remove code like the following from build:

if ( DEBUG ) {
    // Debug code here
}

[Reinmar]

This gives us something... but not too much. Actually, this gives us only what we have in v4 using the CKBuilder's comments. We need much more than that.

[pjasiun]

I agree with @Reinmar. We need some tool which help us better debugging, but also let users give us more information about errors. If we don't want to make final code too big we can use some error codes instead of messages in the build version (end users will no debug it anyway). On the other hand I believe that we could fix some common issue by having good messages in the console.

[pjasiun]

Maybe we could have a debugger as a separate script so have just error codes in the code, but this codes can be automatically changed into exhaustive messages if one wants to and include additional script.

[fredck]

I mean, the DEBUG thing is something we could take in consideration, just that. It's certainly better than the comment thing we have in v4.

Still the way we gonna use it is to be understood. I don't think we need a lot of debugging stuff in the distro code. Maybe just a few error codes linking to online pages with full information and just for the most critical things... the things that users will face commonly.

For the dev team instead, it may be useful to have debugging code being outputted. This time not only for errors, but also for information.

[adelura]

I think since we'll have MV* mechanizm me might adopt assertions and validation into models just to make it more general.

I just want to say that when some value is set, then something is listening change and validate that one. Combining this with event driven approach we don't even need to add some extra code which will be available in DEV mode and removed for production versions. Simply if our debugger is available then errors will be catched and proper information will be displayed in console, if not - nothing happend.

[Reinmar]

Time for some concrete proposal. It's based on http://dev.ckeditor.com/ticket/13632 (please read the discussion). Especially comments 12 and 13.

Module 'ckeditor5-core/log':

• error( errorName, ... additional values ) method,
• warn( errorName, ... additional values ) method.

Usage example:

/**
 * Error thrown when a plugin cannot be loaded due to JavaScript errors, lack of plugins with a given name, etc.
 *
 * @error plugin-load
 */
log.error( 'plugin-load: It was not possible to load the "{$0}" plugin in module "{$1}".', pluginName, moduleName );

In a build that should be automatically replaced with:

log.error( 'plugin-load: {$0}, {$1}.', pluginName, moduleName );

And documentation will be automatically created for defined errors. Plus, in the build version log.error() will log a link to documentation of that error.

Possible issues that I can name right now are:

• Error name collisions. It can be caught when generating documentation, but we can also add additional server-side test (we'll need server-side tests for the builder anyway).
• How to pass additional, non-string arguments. I guess the answer is that arguments not used in the string template will be logged straight to the console, so they can be browsed (e.g. if they are some objects). We can actually load all arguments directly to the console for simplicity (I'll go this way in the first implementation).

Module 'ckeditor5-core/ckeditorerror'

• CKEditorError( errorName, ...additional values ) class (so we can easily and consistently use error names).

These errors will work and be built in the same way as log.error() and log.warn().

[Reinmar]

One thing that may not be clear still is when to use a log.error(), log.warn() and when to throw. I'll write few words about this later.

[pjasiun]

I think we chold consider object as a second parameter:

log.error( 'plugin-load: It was not possible to load the "{$pluginName}" plugin in module "{$moduleName}".', {
    pluginName: pluginName,
    moduleName: moduleName
} );

then the minified version would be:

log.error( 'plugin-load: pluginName: {$pluginName}, moduleName : {$moduleName}.', pluginName, moduleName );

What would be much more useful when one put the plugin name as a module name. Note that the errors message may evolve when we refactor the code and if after refactoring we will log different parameters it may be hard to find what parameter was logged (especially for the number parameters).

Additionally I am not a big fan of messages mining. We could have people complying that we have meaningless error messages and then we would need to explain how to turn full messages on. How cool it would be if you meet an error during the CKE installation and get an message which tell you not only what was wrong but also how to fix it? People really spend hours trying to install editor. Messages should be full and helpful at the begging and having minified messages should be just an option for those who complain on the editor size.

[Reinmar]

Additionally I am not a big fan of messages mining.

Please read the discussion in CKE4's issue tracker.

• Messages minification is a practice used by at least Angular and is totally understandable if you want to be able to log/throw have many errors without worrying for code size.
• You can have a dev plugin which extends minified messages used in the build with full messages that it will automatically get from the docs.
• Finally, why would anyone complain if our methods log links straight to that error message?

Of course some developers will complain, but these developers always complain. I don't want to make bad decisions because of them.

---

As for:

log.error( 'plugin-load: pluginName: {$pluginName}, moduleName : {$moduleName}.', pluginName, moduleName );

That wouldn't work. How do you log it then? What's {$pluginName} if the log.error() have only two strings (ordered, but not named). You can try to think about some tricks to make it work, but they are not worth the effort. KISS! If we'll see any problems we can always change it.

Note that the errors message may evolve when we refactor the code and if after refactoring we will log different parameters it may be hard to find what parameter was logged (especially for the number parameters).

Then it's your fault if you refactored the code incorrectly ;)

[fredck]

As for the minified version, I would go with json: log.error( { error: 'plugin-load', pluginName: 'pluginName', moduleName: 'moduleName' } ). This could be logged as is in the console together with a URL that receives the object keys as parameters.

[pjasiun]

As for:

log.error( 'plugin-load: pluginName: {$pluginName}, moduleName : {$moduleName}.', pluginName, moduleName );
That wouldn't work. How do you log it then? What's {$pluginName} if the log.error() have only two strings (ordered, but not named). You can try to think about some tricks to make it work, but they are not worth the effort. KISS! If we'll see any problems we can always change it.

My bad. I was thinking about:

log.error( 'plugin-load: pluginName: {$pluginName}, moduleName : {$moduleName}.', { pluginName: pluginName, moduleName: moduleName } );

But Freds idea is better.

[Reinmar]

As for the minified version, I would go with json: log.error( { error: 'plugin-load', pluginName: 'pluginName', moduleName: 'moduleName' } ). This could be logged as is in the console together with a URL that receives the object keys as parameters.

We may have name collisions with the error property, so better avoid mixing the error name into the additional vars. Plus, building will be simpler if error name is kept as a separate arg. Other than that I'm quite OK with this idea, although I feel bad with the bigger code size that it will produce. On the other hand, we may not need to use those "additional values" property that often.

[pjasiun]

I think that having named parameters whenever there is more then one would be very useful. Note that you may have whole console of error and need to check what parameters every error contains.