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

如果只获取Response(), 当WithContext在SetTimeout前面时,会导致context cancel错误 #383

Open
aeroplane opened this issue Mar 15, 2024 · 5 comments

Comments

@aeroplane
Copy link

Bug Report

在SSE等流式响应模式中(返回时间很长),需要获取Response

如下代码,会报错

response, err := gout.New().
                WithContext(ctx).
		SetTimeout(100 * time.Second).
		
		SetMethod("POST").
		SetURL(url).
		SetJSON(request).
		Response()
if err != nil {
    panic(err.Error())
}
defer response.Body.Close()

// 读取Response.Body
for {
  _, err :=  response.Body.Read(....)
}

WithContext(ctx)SetTimeout上面时,读取Response.Body(一段时间后)会出现context cancel的异常

产生BUG原因:

func (s *Setting) SetTimeout(d time.Duration) {
	s.Index++
	s.TimeoutIndex = s.Index
	s.Timeout = d
}

// retry模块需要context.Context,所以这里也返回context.Context
func (r *Req) GetContext() context.Context {
	if r.Timeout > 0 && r.TimeoutIndex > r.ctxIndex {
		r.c, r.cancel = context.WithTimeout(context.Background(), r.Timeout)  // 当`WithContext(ctx)`在`SetTimeout`上面时,会执行此行
	}
	return r.c
}

// Response 获取原始http.Response数据结构
func (r *Req) Response() (rsp *http.Response, err error) {
	_, rsp, err = r.getReqAndRsp()
	defer r.Reset()   // 此处会调用r.cancel 
	return
}

因为r.cancel被调用,r.c会直接退出

解决办法

将setTimeout放在withContext上面

SetTimeout(100 * time.Second).
 WithContext(ctx).
		
@guonaihong
Copy link
Owner

guonaihong commented Mar 15, 2024

原因

目前设计是这样的,SetTimeout和WithContext的使用只会有一个生效。后面一个覆盖前面一个。
原因是SetTimeout也是由context实现的。这段代码处理优先级

func (s *Setting) SetTimeout(d time.Duration) {
	s.Index++
	s.TimeoutIndex = s.Index
	s.Timeout = d
}

解决办法-优化提示消息

会出现context cancel的异常

老版本的context.Context实现有个问题,不能使用自定义的错误类型。所以这里的错误有些不友好。

这块我今天或者明天优化下,如果是Timeout的错误就显示"Timeout“, 唯一的遗憾只能 > 1.20的版本这么实现

https://pkg.go.dev/context#CancelCauseFunc

@aeroplane
Copy link
Author

其实传递WithContext(ctx)的原因主要还是外侧代码需要控制goout内部的中断

比如是一个并发任务的控制端

ctx, cancel := context.WithCancel(context.Background,) // 总控制端
if Ctrl +C 被按下 {
  cancel()
}

go func() {
   goout.new().WithContext(ctx).Post(url1)...
}

go func() {
  goout.new().WithContext(ctx).Post(url2)...
}

除了这种场景,还有很多需要外侧控制ctx的场景,比如http.Server中用户主动关闭连接,或者外界代码有timeout的控制要求等

所以SetTimeout后,也应该包裹该ctx才行,比如yourCtx, yourCancel := context.WithTimeout(ctx, 100*time.Second)

在SSE中,需要自行读取Response.Body的内容,建议Wrap下Response.Body,并在Close中封装Reset的相关实现

因为Response.Body.Close方法大部分代码习惯中都会调取的,即使不调取 ,Timeout后,该yourCtx也会内存回收

@guonaihong
Copy link
Owner

guonaihong commented Mar 15, 2024

其实传递WithContext(ctx)的原因主要还是外侧代码需要控制goout内部的中断

比如是一个并发任务的控制端

ctx, cancel := context.WithCancel(context.Background,) // 总控制端
if Ctrl +C 被按下 {
  cancel()
}

go func() {
   goout.new().WithContext(ctx).Post(url1)...
}

go func() {
  goout.new().WithContext(ctx).Post(url2)...
}

除了这种场景,还有很多需要外侧控制ctx的场景,比如http.Server中用户主动关闭连接,或者外界代码有timeout的控制要求等

所以SetTimeout后,也应该包裹该ctx才行,比如yourCtx, yourCancel := context.WithTimeout(ctx, 100*time.Second)

是的,我也打算换成包裹外层的ctx。更合理些。

在SSE中,需要自行读取Response.Body的内容,建议Wrap下Response.Body,并在Close中封装Reset的相关实现

因为Response.Body.Close方法大部分代码习惯中都会调取的,即使不调取 ,Timeout后,该yourCtx也会内存回收

这块我初始的想法是提供一个Callback函数,形参是[]byte,像下面这样,提供sse的功能封装。

func OnSSEData(b []byte) {
}

@aeroplane
Copy link
Author

aeroplane commented Mar 15, 2024

大佬威武

除了SSE外,还有Transfer-Encoding: Chunked的流(用于视频流等领域)
image

或者简单点,由用户自行实现读取

StreamResponse(func(reader io.Reader){
  for {
    reader.Read(...)
  }
})

@guonaihong
Copy link
Owner

ok,我研究下。

@guonaihong guonaihong mentioned this issue May 23, 2024
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

No branches or pull requests

2 participants