Try/catch for 4.x

[miripiruni]

See #298

Original task: https://nda.ya.ru/3RiZp2

[miripiruni]

Benchmark results in web4 https://nda.ya.ru/3RiZii

[miripiruni]

PR: https://nda.ya.ru/3RiZik

[miripiruni]

The only question is: what shall I do with this cases? I think, as long as we introduce try/catch for block rendering it not valid anymore.

[veged]

since it depend on this.options.debug there is should be two ways of tests: with throwing and without

[miripiruni]

Review needed. cc @veged @zxqfox

[St-Lukas]

e.stack?

[miripiruni]

Updated.

[miripiruni]

FOR Minor: if debug !== true ent.run
FOR Maj: this.tryRun

[miripiruni]

Updated. cc @tadatuta @veged

I introduced production flag. If it’s value true bem-xjst will render all templates. If there is an error bem-xjst will log it via console.error.

$ cat stand.js
var bem = require('./');
var template = bem.compile(function() {
  block('page').attrs()(function() {
    var undef = applyNext();
    return undef.test = 'foo';
  });
}, { production: true });
var html = template.apply({ block: 'wrap', content: { block: 'page' } });
console.log(html);


$ node stand.js
BEMXJST ERROR: cannot render block page, elem undefined, mods {}, elemMods {} [TypeError: Cannot set property 'test' of undefined]
<div class="wrap"></div>

This way allows us to maintain backward capability. Also developers do not miss errors, since by default the program will stop its work. We can release it as minor.

[miripiruni]

Notice that there is no documentation in v4.x when I port this feature to v5 and higher I also add documentation.

[miripiruni]

@veged @tadatuta review needed. Thank you!

[veged]

you can return right here without unnecessary res variable

[veged]

LGTM, except one comment

[tadatuta]

wouldn't it be more informative to show whole context:

console.error('BEMXJST ERROR: cannot render ' + JSON.stringify(context, null, 4), e);

also it's possible to get rid of res:

try {
    return ent.run(context);
} catch (e) {
    console.error(/* ... */);
}

return '';

[miripiruni]

@tadatuta context contains circular structure and many information useless for debugging in this case.

[St-Lukas]

e.stack || e, please

[miripiruni]

@St-Lukas e.stack is useless:

93:trycatch miripiruni$ node stand.js
BEMXJST ERROR: cannot render block page, elem undefined, mods {}, elemMods {} TypeError: Cannot set property 'test' of undefined
at ContextChild. (evalmachine.:1863:19)
at Match.tryCatch (evalmachine.:1240:15)
at Match.exec (evalmachine.:1290:16)
at Entity.defaultBody (evalmachine.:266:26)
at Entity.run (evalmachine.:251:15)
at BEMHTML.tryRun (evalmachine.:681:16)
at BEMHTML.runOne (evalmachine.:663:10)
at BEMHTML._run (evalmachine.:531:16)
at BEMHTML.renderContent (evalmachine.:970:18)
at BEMHTML.renderClose (evalmachine.:831:19)

[miripiruni]

JFYI: Pulse test results: https://nda.ya.ru/3RiZii

[miripiruni]

I think we can merge it. And port to 5.x, 6.x, 7.x.

[veged]

yes

[miripiruni]

Also in v5.2.0

[miripiruni]

Also in v6.7.0

[miripiruni]

Also in v7.4.0