Bender swallows errors in CKE5

Reinmar commented 9 years ago

Install https://github.com/ckeditor/ckeditor5
Open http://tests.ckeditor.dev:1030/node_modules/ckeditor5-core/tests/editor/editor
Add throw new Error( 1 ); in https://github.com/ckeditor/ckeditor5-core/blob/master/tests/editor/editor.js#L27
Refresh the test.

You can see that tests failed, but you don't get the error. When I tried to reproduce the same without Bender the error were logged to the console.

scofalik commented 9 years ago

IMO everything works well. In your case. I tested throwing errors in multiple places and it worked as expected. in L27: tests don't even load and I have an error in console in L60: throwed error is caught by require and then handled by plugincollection.js:

function() {
    var err = new Error( 'It was not possible to load the "' + plugin + '" plugin.' );
    err.name = 'CKEditor Error';
    reject( err );
}

in ckeditor5-core/src/plugin.js@25 (before return): I get an error in console and in browser`s HTML.

When I tried to reproduce the same without Bender the error were logged to the console.

What does this exactly mean? I tried to play with the code and see what will be thrown when I put throw in various places and everything went according to the plan. You provided me with some HTML sample but it wasn't really corresponding what was happening in the linked test. Here is what I came up with:

<!DOCTYPE html>
<html>
<head>
    <meta charset="utf-8">
    <title>Sample</title>
    <script src="../node_modules/requirejs/require.js"></script>
    <script src="../ckeditor.js"></script>
</head>
<body>

<script>
'use strict';

require( [ 'ckeditor', 'editor' ], function( CKEDITOR, Editor ) {
    var editor,
        element = document.createElement( 'div' );

    document.body.appendChild( element );

    editor = new Editor( element, {
        plugins: 'A'
    } );

    function pluginDefinition( name ) {
        return function( Plugin ) {
            throw new Error(22);
        };
    }

    CKEDITOR.define( 'plugin!A', [ 'plugin' ], pluginDefinition( 'A' ) );

    editor.init();
} );
</script>

</body>
</html>

I more-or-less recreated the test. And I end up in the same place as in test -- plugincollection.js error handling. So this does not look like an error in Benderjs. Please provide a better example or I will close this issue.

Reinmar commented 9 years ago

If plugincollection.js handled that error fully, then we would have the same behaviour in Bender and without it. But it's very different.

I'll try to debug it myself. Stay tuned.

Reinmar commented 9 years ago

OK, turns out there are two issues.

First thing is that plugincollection in CKE5 turns a meaningful error into a totally useless "Cannot load that" error. I'll fix this in CKE5.
Second things is that Bender does not log stack of the error in this case. It always log stacks when an error occurs in a TC and it should do this now too.

scofalik commented 9 years ago

I'll check logging the stack.

scofalik commented 9 years ago

This is mocha's thing. If you want full stack in errors you have to put this in bender.js configuration:

mocha: {
    fullTrace: true
}

Since this resolves error number 2 (and had to be changed in CKE5) and error number 1 is connected with CKE5, I am closing this issue.

Reinmar commented 9 years ago

Don't you think that this option should be set to true by default?

scofalik commented 9 years ago

I can get behind it. Still, this is benderjs-mocha issue.

benderjs / benderjs

Bender swallows errors in CKE5 #245