aralejs / events

Events Utility
aralejs.org/events/
34 stars 41 forks source link

关于trigger时异常处理的建议 #1

Closed huipengyu closed 11 years ago

huipengyu commented 11 years ago

Hi,aralers :) 最近考虑使用arale的events,但发现在trigger的时候,对for循环中的apply没有异常捕获。这样的话,如果注册上来的某个callback出现异常,那么callback list中后续的所有回调函数都将得不到执行。

个人认为这样不够合理,所以有后续建议,仅为抛砖。apply放入try catch中,出现异常立即处理,考虑以下处理办法

  1. log输出warn或error,再移除异常callback,也可考虑回调处理error的callback
  2. catch 到error后,再defer抛出,这样至少不会影响后续执行,同时不丢失error信息

注:原来在写页游时遇到的一个情景:我们游戏的战斗逻辑等各种复杂逻辑是通过events来处理的,这样效果很好,当所有逻辑与渲染开发完毕后,通过event的形式挂载了音效模块,即对各种战斗事件挂载播放不同音乐。但如果音乐播放出现异常,基于event的其他游戏逻辑不会受到影响。 注:本来想写个test验证一下,但是tests中的runner跑不起来,http://aralejs.org/tools/seajs-helper.jshttp://aralejs.org/tools/jasmine-runner.js 都是404。spm用的不是很熟,是不是我用错了。

lifesinger commented 11 years ago

加 try catch 不是很合适,因为 JavaScript 的单线程特性,导致 events 实现时是顺序调用 handlers 的,这些 handlers 之间可能有顺序依赖,当前面的出错时,再继续调用后面的,后面的状态已经可能不对,就如种田一样,如果插秧这一步出了问题,再往后执行状态已经不可控,非预期了。

针对页游这个例子,感觉应该是每一个有可能出错的 handler 自身应该考虑到自己出错,出错时,内部有 try catch 处理。具体实现可以通过父类来统一处理,所有有可能出错的 handler,继承自同一个父类,父类里集中处理好就行。

lepture commented 11 years ago

@huipengyu

注:本来想写个test验证一下,但是tests中的runner跑不起来,http://aralejs.org/tools/seajs-helper.jshttp://aralejs.org/tools/jasmine-runner.js 都是404。spm用的不是很熟,是不是我用错了。

因为项目调整过很多次,这个文件已经过时。请参考 http://aralejs.org/events/tests/runner.html

huipengyu commented 11 years ago

感谢反馈,但大家的作息不太健康啊~ :) @lifesinger @lepture

理解尊重玉伯的观点,但仍建议未来考虑异常捕获,三点原因: 1 当前端的能力越来越强,开发的app越来越重大复杂,用户打开某个app后的操作停留时间也可能越来越长,很容易产生错误状态的累积,造成用户不得不重新刷新页面。而且有些时候,一些蛋疼的异常是不易预期的,在events中加入try catch是可以增强复杂app鲁棒性的。 2 我觉得这样是同HTML原生的event机制更一致

    document.addEventListener("keydown",function(e){
        throw new Error();
        console.log("第一个", e.keyCode); 
    });
    document.addEventListener("keydown",function(e){
        console.log("第二个", e.keyCode); //这里会不受其它handler中异常的影响
    });

3 在 issue#2 中,玉伯提到event更像广播,各个handler没有顺序,互不影响,这点我非常同意。try catch就是对这点更好的保证。

huipengyu commented 11 years ago

补充楼上第1条: 现在的大部分页面或者app,操作的复杂性比较小,用户在单页上产生几十个操作就很不错了,再复杂点也很少超过百次以上的操作,用户就会跳转页面,消除累积的错误。 现在canvas 或者 webgl 的应用,甚至未来以后结合websocket,可以多人同步。这样的应用则是希望用户可以打开页面后就不会刷新,用户可能积累千次或更高的操作次数。

lifesinger commented 11 years ago

错误捕捉这个我考虑下,是个经典问题了,可以看下明城翻译的这篇文档:

http://www.gracecode.com/posts/2940.html

我仔细考虑下。

huipengyu commented 11 years ago

很棒的文章,分析的很全面。 很期待能处理这个问题。 回调callback list这部分做切面做不进去,如果我们使用者要实现这个需求,就只能通过改源码,实在不想这么处理。 如果能支持个类似策略模式的概念, 让我们能把自己的策略填进去;又或者让外面有能力在这个位置做切面(重写apply,外面能碰到),也是很不错的,就不至于改源码了。

感谢对这个问题的关注。

popomore commented 11 years ago
try{
  callback()
} catch(e) {
  console.error(e.stack)
} finally {
  continue;
}

感觉用这种方式就够了,捕获到错误后打印出来(不支持 console 的不输出),并中断当次循环,继续下一个循环。

@lifesinger

lifesinger commented 11 years ago

这个还是别加啊

我很同意 backbone 作者的这句话:

Suppressing errors inside event handlers is a much worse behavior than skipping the rest of the handlers. When something fails, you want to know immediately, not continue as though nothing happened.

Backbone 社区有过讨论,最后是决定不处理:

https://groups.google.com/forum/#!msg/backbonejs/bvUVuovKfms/F8kJZqFeiU0J

https://github.com/documentcloud/backbone/issues/1527#issuecomment-7554619

hotoo commented 11 years ago

我不太认同 @lifesinger 的观点。

觉得不同 handler 之间不应该出现依赖(原生事件机制也是无序的),因此不同 handler 应该也是在独立作用域中,抛出的异常不应该其他 handler。

明城的博客分析的很棒,如果是站在结果的角度,最喜欢基于事件的机制,独立的作用域可以保证结果和原生及期望的一致,而且异常抛出的异常可以被统一监控捕获。缺点是复杂了点,逻辑和性能等都是需要考量的(try/catch 在捕获到异常时也有性能问题)。

但是 finally 的方案也可以接受,简单实用。前端监控中提供了 monitor.error(Error ex) 这样的 API,建议使用。

p.s. 如果不 rethrow ex,finally 貌似没什么必要。但是 rethrow 的异常不能被浏览器捕获(亦不触发 window.onerror),意义不大。

popomore commented 11 years ago

之前跟 @lifesinger 讨论过,希望 events 尽可能的保持简单,而且用户的使用场景无法预知,比如有些场景就需要第一个报错而停止之后的执行。

使用者也可以对 callback 再做一层封装。

obj.on('a', wrap(callback))

function wrap(fun) {
  return function() {
    try {
      fun.apply(this, arguments);
    } catch(e) {}
  }
}
hotoo commented 11 years ago

不认同。

  1. 更多场景是希望各个 handler 互不影响,不同注册者尤其如此。
  2. handler 有依赖的场景不应该出现的,这个可以在文档中注明,甚至说不保证触发的顺序。

确实需要依赖的,考虑:

  1. 放到同一个 handler 中。
  2. 使用标记。
var flag = false;
obj.on("a", function(){
  try{}catch(ex){
    flag = true;
    throw ex; // monitor.error(ex);
  }
});
obj.on("a", function(){
  if(flag){return;}
  // XXX
});

觉得这个保持简单有点一厢情愿,而用户的正当需求却没有被满足。

lifesinger commented 11 years ago

@hotoo 我们的观点在基本层面是一致的:

events 应该是天女散花,无顺序,无依赖。任何 handler 都不应该有顺序依赖性。我一直非常强调这一点。

但是无顺序依赖,并不代表无其他依赖,比如

  1. handler A 更新了数据库,添加了一个新用户
  2. handler B 需要查询数据库,获取总用户数

比如上面这种情况,因为空间的共享性,实质上会让 handler 之间是有依赖的。

同样的,对于前端开发来说,由于共享了同一个 DOM 树,实际上 handler 之间也是有可能互相影响,很难从理论上做保证完全不依赖。

这样,用 try catch 去保证无依赖性,实质性却并不能保证,反而可能引起一连串不正确的操作。

就如 Backbone 作者所说的那样:

Suppressing errors inside event handlers is a much worse behavior than skipping the rest of the handlers. When something fails, you want to know immediately, not continue as though nothing happened.

与停止执行相比,在事件处理器中抑制错误是一种更糟糕的行为。当某些事情不对时,就应该立刻知道,而不是装着什么也没发生一样继续执行。

不加 try catch,跟保持简单性没半毛钱关系,而是为了更正确的处理问题。

某个 handler 或一批 handler 是否应该在抛错时不影响其他 handler 的执行,这是 user-land 的范畴,可以自己封装。倘若 events 层封装的话,反而剥夺了用户的选择权,使得用户不再能选择抛错不执行的策略。不封装的话,用户想怎么着都行,更拥有自由。

popomore commented 11 years ago

两种方式的分歧点并不是“能不能发现错误”,而是一旦出错会不会影响其他 callback。现在的这种做法是将权利交给使用者,使用者可以按照自己的情况去进一步封装。

afc163 commented 11 years ago

其实吧,这个功能,做成啥样使用者就怎么用,不会有人去蛋疼的加一个try catch的,这不是事前明知自己要出错。

hotoo commented 11 years ago

@lifesinger

最重要的是两个不同 handler,A 更新数据库或 DOM 失败或什么原因造成的异常,跟 B 是没有关系的,除非它明确对 A 的这个更新有依赖。不依赖 A 的 B 应该可以继续完成它的工作。

浏览器原生的类似案例:

  1. 多个 script 间的异常不相互影响
  2. 多个事件绑定间的异常不相互影响

另外,我上面提到的第 2种『使用标记』法,也是一种交给用户的方法。只不过是反过来了,默认认为各 handler 无依赖,如果你们有依赖,需要你们自己配合解决。

上面举的例子没能说服我,还可以举其他例子吗?

lifesinger commented 11 years ago

仔细想了下,还是得分两个场景来说:展现型页面和功能型页面。

对于展现型页面来说,比如淘宝首页,页面某一个区块出问题时,最好不影响另一个区块的展现,因为一般来说,各个区块之间不会有强关联。感觉这也是浏览器设计之初,独立 script 之间互不影响的初衷。

对于功能型页面来说,比如 Gmail,当页面某一个区域出问题时,经常意味着底层数据或网络出了问题,这时最好的处理方式是,都停下来,统一给出错误或重试提示,而不是继续进行操作,因为操作已经不可预期,很可能造成不必要甚至错误的操作,比如发了一封错误的邮件等。

Backbone 使用场景应该是功能型页面,因此非常坚持出了任何错误,都不再继续执行后续 handler 的策略。类似的 YUI3 也是如此。

对于支付宝来说,由于支付操作涉及用户金额,有可能存在以下可能性:

  1. handler A 检查校验码,有可能通过,有可能不通过,成功时,会设置某个数据为 pass
  2. handler B 提交支付请求,提交前会依赖校验数据

当 handler A 出错时,校验数据有可能停留在过去值,也有可能被设置成错误值 handler B 并不依赖 handler A,但依赖校验数据,当 handler A 出错时,校验数据无论是什么值,都已经不可靠,即便是校验通过,也不应该提交支付请求。

这就是说,对于功能型页面来说,一旦有代码错误(不一定是 handler 引发的),就应该尽可能做到停止代码执行,并告知用户出了问题,可以刷新页面重试。

这就如一锅汤,一旦滴进了一滴毒药,只要发现有一个人中毒了,最明智的做法就是立刻不再继续把汤盛给其他人,否则毒死一批人,罪孽就大了。

问题的核心是,要判断滴进汤里的是毒药,还是仅仅是一粒沙子。对展现型页面来说,经常是沙子,无伤大雅,但对功能型页面来说,我情愿假设都是毒药,应立刻告知所有人并停止喝汤。

lifesinger commented 11 years ago

Arale 定位为面向支付宝的前端解决类库,以后会有更多金融型产品出现,都是偏功能型的,因此我觉得还是假设都是毒药的好。

对于展现型的页面,若基于 Arale 开发,如果想确保不被一粒老鼠屎坏掉一锅汤,可以自己再次封装下。

events 本身还是尽量少做一些功能,少其实是为了多。倘若多做了一层处理,反而能支持的场景变少了。

hotoo commented 11 years ago
  1. 还是觉得让有依赖的 handlers 使用状态标记来自己保证执行顺序、异常处理、状态变更比较合理。
  2. 各个 handlers 只依赖 trigger,只要被 trigger,就没有理由不执行。
    1. 演讲者说话了,旁边的聋子不应该影响到我。
    2. 除非我是聋子,依赖旁边的手语助手。
    3. 如果助手没听清,或者打了我没看懂的手势,我可以和助手进行协同,因为我明确知道自己依赖谁。
    4. 反之,要求每个听众找到潜在的聋子,说你别影响我,这太不合理了。而且如果聋子无动于衷,其他听众也无能为力。
    5. 总之,广播不因某个听众而影响其他听众,如果某个听众没听清,找旁边的依赖听众重述一遍,让这两个听众自己协同好了。
  3. 举的例子都太虚了,上面的例子都是空想出来的,实际这两个 handler 不会注册同一个事件(即使有,参考第一条)。

所以,我还是坚持默认为监听者间不依赖。需要依赖的监听者自行协同。支持的场景并没有减少,反而是更多了(可以和已知的依赖者协同)。

lifesinger commented 11 years ago

就是不依赖呀,这条是一致的,没有分歧的

lepture commented 11 years ago

其实这个问题很简单处理嘛:

events.suppress = true
events.on('a', fn)

非 suppress 下,用户自己 try catch

events.suppress = false
events.on('a', function() {
   try {
   } catch (e) {}
})
lifesinger commented 11 years ago

@lepture 你这是两面派,徒增了一点点不必要的复杂性

huipengyu commented 11 years ago

认真读了<事件触发的一个细节设计上下>, @lifesinger 举的两个应用情境,功能型和展现型,来对应停止策略与继续策略。 那么最后的选择是:选择停止策略,一方面出于对“支付宝”的考虑,另一方面try catch 属于user-land, 可以业务层面自己去处理。 上面是我对分析过程与选择结果的理解。 “观点没有对错,只是角度不同”, 站在相应的角度,观点都是对的。如果观察的角度不一致(价值观不同),那么结果就不指望相同。 所以,对于这两种策略,没必要再继续讨论对错, 而是要找到共同的角度(共同的价值观)。

如果站在灵活性普适性的角度,是值得继续挖掘的。“少即是多”理念OK,但如果解决办法是“仅可以使用者对自己的业务做try catch”,我认为这是arale.js/events能力上的不健全,可以考虑再做一些改动。我的一些观点如下:

  1. 支持多种策略:例如@lepture 的办法或类似的,玉伯观点说是”两面派,增加了一点点复杂性“, 有道理。但个人认为稍微有点”洁癖“了,虽然破坏的一点点原则, 但是一下子能力就变大了许多,个人认为值得的。海纳百川有容乃大,包容性也包括对瑕疵的包容。
  2. 或者允许其它策略的注入:我认为这是最起码的,比如让外面可以做切面aop,把策略做进去,但是现在对于callback list 的遍历处理是在for循环的原子操作中直接写的, 对event.on没法做aop或其它扩展,只能改源码代码。

p.s. 分享一下我们应用情景,拓展大家思路:我做的一个游戏开发过程是这样了, 先做基本的功能流程开发,event-based, 例如”游戏胜利“这个环节,逻辑开发好后,产品说需要挂特效,用event 就很方面,event.on("win",function(){/全屏动画特效/}); event.on("win",function(){/声音特效/}); 如果终止问题来了,用户如果声音解码有问题,出现异常,那么就看不到胜利画面了, 甚至不能胜利。 我们经历的场景中,这种不可预见的问题很多,而且这种”老鼠屎“级别的问题, 比”毒药“级别的问题要多得多。

lifesinger commented 11 years ago

@huipengyu 针对你最后提到的应用场景,我的想法是:

event.on("win", errorProne(function() { /* 声音特效 */ }))
event.on("win", errorProne(function() { /* 全屏动画特效 */ }))

通过 errorProne 方法,来将有可能出错的 handler 再封装一层。

popomore commented 11 years ago

这个 issue 就讨论到这吧,events 是广播式触发的,每个 handler 之间是无依赖的,这个大家都是认同的。

events 还是保留“少即是多”的理念,给开发者更大的空间。如果需要处理异常可根据 @lifesinger 上面说的再进行一层封装。