crimx / observable-hooks

⚛️☯️💪 React hooks for RxJS Observables. Concurrent mode safe.
https://observable-hooks.js.org
MIT License
1.03k stars 44 forks source link

useSubscription 会触发 JS 报错导致页面白屏 #89

Closed hulh122 closed 2 years ago

hulh122 commented 3 years ago

描述

我们项目在遇到 http 报错的时候整个页面会白屏。

原因分析

useSubscriptionInternal 中在调用方未申明 subscribe 的 error handle 的时候,会把当前的 error throw 出来,触发了 JS 报错,导致页面白屏 - 在项目 use-subscription-internal.ts 的 90-93 行,备注了为了项目可以用 error boundary catch。

可以用这个例子查看

为什么修改

在 RXJS 的原生行为中,如果触发了 error 应该在 catchError 这个 operator 或者 subscribe 的 error handle 阶段去处理,本身的订阅不会触发 JS 报错而是 console.error 打印出 error。而 useSubscription 的这种行为会导致我们需要在所有调用 service 的地方加上 catchError,否则一旦接口出现问题,就会导致页面直接白屏。

如何修改

可以考虑加配置来供业务方配置是否需要 throw error,也可以考虑让业务方自行处理 RX 中的 error:即取消这一段 throw error 的代码

crimx commented 3 years ago
useSubscription(a$, { error: console.log });

如果提供了 error observer 不会抛出错误的。

https://github.com/crimx/observable-hooks/blob/cd7414e0f999e87aa798816ef6582e98afc683da/packages/observable-hooks/src/internal/use-subscription-internal.ts#L59-L65

crimx commented 3 years ago

有三个地方处理异常:

  1. Observable 是业务方提供的,那么可以直接在 Observable 上 catchError。
  2. 通过 useSubscription 的 error observer 接收异常。
  3. 如果 Observable 上的异常没有得到处理,那么 useSubscription 会将异常抛给 ErrorBoundary,给了业务方在 React UI 上处理的能力。

observable-hooks 作为一个基层库,不能鼓励静默吞掉异常,故没有提供隐藏错误的便利接口。业务方有这个需求可以包一层 hook 处理。

hulh122 commented 3 years ago

我们目前遇到的问题是 http 的报错转化为了 js 报错

对于 http 接口默认是不应该相信的,要做到这点只能在每个接口处包装 catchError 的逻辑【这个影响范围很大】

我疑惑的点在于 useSubscription 应当是对 useEffect过期 state 的包装 hook,但加入 ErrorBoundary 的处理后实际上改变了 Rx.subscribe 对于错误的原生处理行为。我理解异常不是被吞掉了,而是包裹在了 Rx 事件流的本身

还有个问题,对于 useObservableState 这个 hook,其原代码中调用 useSubscription 没有增加 error 处理,这导致我在使用这个 hook 包裹 http 请求时还只能在 observable 中处理 catchError

yanhaijing commented 3 years ago

如果 Observable 上的异常没有得到处理,那么 useSubscription 会将异常抛给 ErrorBoundary,给了业务方在 React UI 上处理的能力

这个行为感觉不太符合rxjs 的逻辑,rxjs的中的所有js报错都被rxjs劫持了,通过observer的error来处理,这才是rxjs推荐的错误处理方式

再把rxjs的错误,暴露到js中,这行为不是让rxjs的工作都白做了

业务方要处理报错,应该自己订阅error事件是比较合理的吧

crimx commented 3 years ago

如果 Observable 上的异常没有得到处理,那么 useSubscription 会将异常抛给 ErrorBoundary,给了业务方在 React UI 上处理的能力

这个行为感觉不太符合rxjs 的逻辑,rxjs的中的所有js报错都被rxjs劫持了,通过observer的error来处理,这才是rxjs推荐的错误处理方式

再把rxjs的错误,暴露到js中,这行为不是让rxjs的工作都白做了

业务方要处理报错,应该自己订阅error事件是比较合理的吧

第三点的前提正是业务方没有订阅error处理(第二点)。否则这个异常触达hooks了,业务方该如何处理?

hulh122 commented 3 years ago

Rx 里的异常处理在 Rx 内部自行处理,如 catchError 处理或订阅 error 处理。业务方决定是否抛出 error 交付给 ErrorBoundary 处理

crimx commented 3 years ago

我们目前遇到的问题是 http 的报错转化为了 js 报错

对于 http 接口默认是不应该相信的,要做到这点只能在每个接口处包装 catchError 的逻辑【这个影响范围很大】

我疑惑的点在于 useSubscription 应当是对 useEffect过期 state 的包装 hook,但加入 ErrorBoundary 的处理后实际上改变了 Rx.subscribe 对于错误的原生处理行为。我理解异常不是被吞掉了,而是包裹在了 Rx 事件流的本身

请看回上面提的三点方式,最后一步是业务方没有处理 rxjs error 才会抛出的。否则 error 触达 hooks 后有其它方式能让业务方 catch 到吗?

还有个问题,对于 useObservableState 这个 hook,其原代码中调用 useSubscription 没有增加 error 处理,这导致我在使用这个 hook 包裹 http 请求时还只能在 observable 中处理 catchError

useSubscription 接收 error 回调以及 observer 对象,文档有说明。如果你依赖的 hooks 没有做这个处理,那么应该是这个 hooks 的 bug,缺失异常处理。

crimx commented 3 years ago

Rx 里的异常处理在 Rx 内部自行处理,如 catchError 处理或订阅 error 处理。业务方决定是否抛出 error 交付给 ErrorBoundary 处理

我不太明白,这不就是现在的处理方式么?

crimx commented 3 years ago

如果不想抛出,只要包一层 hooks 挂上 error observer 吞掉异常就行。

crimx commented 3 years ago

我总结一下,observable-hooks 抛出 error 到 react 的原因是库本身必须是自洽的,出现了异常必须要给下游捕捉的能力。

你的建议我理解下来是希望加一个开关,控制是否抛出异常到 react 层。但在目前 useSubscription 的接口设计上不会考虑加一个 param 的方式,所以即便是做也是包一层 hooks 的处理方式。

最后加一个吞掉异常的方法在我看来加到基层库上不是很合适,所以建议可以自行实现即可。

crimx commented 3 years ago

这样:

function useSilentSubscription(input$, observerOrNext, error, complete) {
  const next = observerOrNext?.next || observerOrNext;
  error = observerOrNext?.error || error || console.error;
  complete = observerOrNext?.complete || complete;
  return useSubscription(input$, next, error, complete);
}
hulh122 commented 3 years ago

我们在业务里大多使用的其实是 useObservableState,这样的话我们只能重写这两个 hook 了。

至于自洽这个问题,我们这边理解 Rx 本身的行为就是会吞掉这个报错,而不是让下游 try catch 这个 error。不同的点就在于,同样一个 Observable,在出错时,我用 subscribeuseSubscription 他们的行为不一致。一个不会抛出错误到 Js,一个会。

如果作者这边不希望修改这个问题的话,我们这边目前打算在自己的 scope 里关掉这几行代码,不然我们需要重写两个 hook,脱离了使用这个库本身的初衷。

hulh122 commented 3 years ago

另外,如果不是考虑到网络请求的问题,我们非常理解你这么设计的原因。

还请考虑考虑如果是网络请求导致的错误,业务方在使用这两个 api 时如何处理这种情况。

crimx commented 3 years ago

这么说抛出异常其实可以放在 useObservableState 上而不是 useSubscription,因为 useSubscription 已经有了途径捕获异常。

hulh122 commented 3 years ago

是这个意思,useObservableState 是没有捕获能力的,而 useSubscription 是有 error handle 可以处理的。

但是直接在 useObservableState 抛出异常依然无法解决网络请求的问题,它需要我们在每个请求点 catchError。

是否可以针对网络请求单加一个 hook ?

crimx commented 3 years ago

那有个问题,如果不 catchError 你们的网络请求出问题了如何处理?

hulh122 commented 3 years ago

我们自己在请求层用 interceptor 封装了对 http 错误的处理,右上角会有对应的弹窗展示错误,如果想在下游自定义错误也可以用 catchError,如果我们想用 errorBoundary 类似对 UI 层的处理,也可以在 catchError 中定义一套错误展示的 UI,用 state 替代 js 报错来展示 ErrorUI

crimx commented 3 years ago

这样可能得上一个 breaking change,默认不抛出流异常到 Error Boundary,增加 useRenderThrow 来抛出。

useObservableState(useRenderThrow($input))
hulh122 commented 3 years ago

^^ 如果能这样就最好了

hulh122 commented 3 years ago

有一个疑惑是 Rx 里所有的报错都会被内部 catch,除了在订阅阶段应该都没办法 throw error。这个 useRenderThrow 是对错误加一个标志位然后在 useSubscription 中处理吗?这一点我不是很理解。

crimx commented 3 years ago

我的思路是 useRenderThrowpipe 一个 catchError 来处理异常。

hulh122 commented 2 years ago

useRenderThrow(ob$, (err) => {}) 这样处理应该可以

crimx commented 2 years ago

这种方式需要两次订阅,捕捉错误一次,使用一次,就需要 Observable 是热或者纯的,否则两次订阅可能出现不一致结果。

hulh122 commented 2 years ago

useRenderThrow 不是要订阅两次的意思

它的实现就是 .pipe(catchError(() => { // 执行 error 回调 }))

要明确一个点是:除了订阅之后,没有办法可以把错误抛到 JS 层,因为 Rx 内部都把错误 catch 住了。那这个 hook 加不加都差不多,都需要自定义 catch 逻辑,不如业务自己加 catchError

crimx commented 2 years ago

useRenderThrow 不是要订阅两次的意思

它的实现就是 .pipe(catchError(() => { // 执行 error 回调 }))

要明确一个点是:除了订阅之后,没有办法可以把错误抛到 JS 层,因为 Rx 内部都把错误 catch 住了。那这个 hook 加不加都差不多,都需要自定义 catch 逻辑,不如业务自己加 catchError

那这个接口跟现在的 useSubscription($input, { error: () => any }) 功能一样呐哈哈哈😂。

useRenderThrow 实现了,麻烦 review 一下 #90 。

hulh122 commented 2 years ago

very nice 👏

crimx commented 2 years ago

observable-hooks@4.2.0 已发布。