Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix crashing issue when connection is reset #427

Closed
wants to merge 2 commits into from

Conversation

laysent
Copy link

@laysent laysent commented Sep 23, 2018

应该可以解决 #408 #372 #370#410
重现方法:系统是 Mac 或 Windows 都可以,node 需要 v10。开启代理之后,访问一个页面,然后在加载完毕前访问新的页面或刷新,应该就会触发 ECONNRESET 的未捕获异常。
之前 node 8 版本是可以正常运行的,但是 node 10 不行,很可能是因为 node 10 引入了这个 commit:nodejs/node@6f3a177#diff-e04bb2b2c80feabbca11f04983e000cd 导致原先默认的 error 事件回调缺失,报错成了未捕获异常。

@CLAassistant
Copy link

CLAassistant commented Sep 23, 2018

CLA assistant check
All committers have signed the CLA.

attend to fix issue alibaba#370, alibaba#372, alibaba#408 and alibaba#410
according to commit e04bb2b2c80feabbca11f04983e000cd in node,
there is no default error event listener for socket,
and should provide one by client for error handling.
attend to fix issue alibaba#391
throw error when accessing cache file that is not inside cache folder
@laysent
Copy link
Author

laysent commented Sep 24, 2018

第二个提交试图解决 #391 的问题,在 cache 文件不在 cache 目录下的时候,访问会报错

@laysent
Copy link
Author

laysent commented Oct 8, 2018

@codingfishman 这个 PR 可以麻烦 review 一下吗

@ajsharp
Copy link

ajsharp commented Nov 19, 2018

Are there plans to merge this? I can confirm this fixes the crash on node 10+. Node 8.x is not affected.

@infoneershalin
Copy link

I am facing this issue on Node 10, when can we have release of this fix?

@songkeys
Copy link

songkeys commented Jan 6, 2019

I'm also suffering the crash. Please review this commit?

@ottomao ottomao closed this in 9682926 Feb 26, 2019
@ottomao
Copy link
Member

ottomao commented Feb 26, 2019

We've just released v4.0.13 to fix this. You may have a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants