Repository: cloudwego/hertz
Pull request #1010 (2023-11-29 11:43:56)

optimize: remove short connection error in trace
#### What type of PR is this?
optimize

#### Check the PR title.
- [x] This PR title match the format: \<type\>(optional scope): \<description\>
- [x] The description of this PR title is user-oriented and clear enough for others to understand.
- [ ] Attach the PR updating the user documentation if the current PR requires user awareness at the usage level. [User docs repo](https://github.com/cloudwego/cloudwego.github.io)


#### (Optional) Translate the PR title into Chinese.
不再在 trace 中记录短连接这个“错误”

#### (Optional) More detailed description for this PR(en: English/zh: Chinese).
<!--
Provide more detailed info for review(e.g., it's recommended to provide perf data if this is a perf type PR).
-->
en:
zh(optional):

#### (Optional) Which issue(s) this PR fixes:
<!--
Automatically closes linked issue when PR is merged.
Eg: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->

#### (Optional) The PR that updates user documentation:
<!--
If the current PR requires user awareness at the usage level, please submit a PR to update user docs. [User docs repo](https://github.com/cloudwego/cloudwego.github.io)
-->

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/cloudwego/hertz/pull/1010?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`a327795`)](https://app.codecov.io/gh/cloudwego/hertz/commit/a3277952e0e05db44f51757431ff28b90bf87736?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) 82.48% compared to head [(`d3a9273`)](https://app.codecov.io/gh/cloudwego/hertz/pull/1010?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) 82.57%.


<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           develop    #1010      +/-   ##
===========================================
+ Coverage    82.48%   82.57%   +0.08%     
===========================================
  Files           98       98              
  Lines         9964     9974      +10     
===========================================
+ Hits          8219     8236      +17     
+ Misses        1249     1242       -7     
  Partials       496      496              
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cloudwego/hertz/pull/1010?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).


Patch:
diff --git a/pkg/protocol/http1/server.go b/pkg/protocol/http1/server.go
index c17aa0f..a5d33f1 100644
--- a/pkg/protocol/http1/server.go
+++ b/pkg/protocol/http1/server.go
@@ -114,7 +114,7 @@ func (s Server) Serve(c context.Context, conn network.Conn) (err error) {
 
 	defer func() {
 		if s.EnableTrace {
-			if err != nil && !errors.Is(err, errs.ErrIdleTimeout) && !errors.Is(err, errs.ErrHijacked) {
+			if shouldRecordInTraceError(err) {
 				ctx.GetTraceInfo().Stats().SetError(err)
 			}
 			// in case of error, we need to trigger all events
@@ -460,3 +460,23 @@ func (e *eventStack) pop() func(ti traceinfo.TraceInfo, err error) {
 	*e = (*e)[:len(*e)-1]
 	return last
 }
+
+func shouldRecordInTraceError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	if errors.Is(err, errs.ErrIdleTimeout) {
+		return false
+	}
+
+	if errors.Is(err, errs.ErrHijacked) {
+		return false
+	}
+
+	if errors.Is(err, errs.ErrShortConnection) {
+		return false
+	}
+
+	return true
+}
diff --git a/pkg/protocol/uri_windows.go b/pkg/protocol/uri_windows.go
index 2e0bf9d..a40daef 100644
--- a/pkg/protocol/uri_windows.go
+++ b/pkg/protocol/uri_windows.go
@@ -67,7 +67,7 @@ func checkSchemeWhenCharIsColon(i int, rawURL []byte) (scheme, path []byte) {
 
 	// case :\
 	if i+1 < len(rawURL) && rawURL[i+1] == '\\' {
-		return nil, rawURL    
+		return nil, rawURL
 	}
 
 	return rawURL[:i], rawURL[i+1:]


Test patch:
diff --git a/pkg/protocol/http1/server_test.go b/pkg/protocol/http1/server_test.go
index 5f8f782..dc8790a 100644
--- a/pkg/protocol/http1/server_test.go
+++ b/pkg/protocol/http1/server_test.go
@@ -443,3 +443,13 @@ type mockErrorWriter struct {
 func (errorWriter *mockErrorWriter) Flush() error {
 	return errors.New("error")
 }
+
+func TestShouldRecordInTraceError(t *testing.T) {
+	assert.False(t, shouldRecordInTraceError(nil))
+	assert.False(t, shouldRecordInTraceError(errHijacked))
+	assert.False(t, shouldRecordInTraceError(errIdleTimeout))
+	assert.False(t, shouldRecordInTraceError(errShortConnection))
+
+	assert.True(t, shouldRecordInTraceError(errTimeout))
+	assert.True(t, shouldRecordInTraceError(errors.New("foo error")))
+}