Repository: cloudwego/hertz
Pull request #1332 (2025-04-14 07:31:38)

fix(server): Shutdown checks ExitWaitTimeout
#### What type of PR is this?
fix

<!--
Add one of the following kinds:

build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
docs: Documentation only changes
feat: A new feature
optimize: A new optimization
fix: A bug fix
perf: A code change that improves performance
refactor: A code change that neither fixes a bug nor adds a feature
style: Changes that do not affect the meaning of the code (white space, formatting, missing semi-colons, etc)
test: Adding missing tests or correcting existing tests
chore: Changes to the build process or auxiliary tools and libraries such as documentation generation
-->

#### Check the PR title.
<!--
The description of the title will be attached in Release Notes, 
so please describe it from user-oriented, what this PR does / why we need it.
Please check your PR title with the below requirements:
-->
- [ ] This PR title match the format: \<type\>(optional scope): \<description\>
- [ ] 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.


#### (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: make sure `Shutdown` will not block longer that `ExitWaitTimeout`
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/1332?dropdown=coverage&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:
> Project coverage is 72.68%. Comparing base [(`67fe3c7`)](https://app.codecov.io/gh/cloudwego/hertz/commit/67fe3c7837092580627a2b3f45f8adf26304a694?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego) to head [(`d81fd5b`)](https://app.codecov.io/gh/cloudwego/hertz/commit/d81fd5bfa7726208db5f242ffde02bfe27554c4c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=cloudwego).

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


```diff
@@             Coverage Diff             @@
##           develop    #1332      +/-   ##
===========================================
- Coverage    72.80%   72.68%   -0.12%     
===========================================
  Files          139      139              
  Lines        16244    16245       +1     
===========================================
- Hits         11826    11808      -18     
- Misses        3823     3840      +17     
- Partials       595      597       +2     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/cloudwego/hertz/pull/1332?dropdown=coverage&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).

<details><summary> :rocket: New features to boost your workflow: </summary>

- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>

Patch:
diff --git a/pkg/app/server/hertz.go b/pkg/app/server/hertz.go
index b4a3f9c..b97fd17 100644
--- a/pkg/app/server/hertz.go
+++ b/pkg/app/server/hertz.go
@@ -73,12 +73,7 @@ func (h *Hertz) Spin() {
 		return
 	}
 
-	hlog.SystemLogger().Infof("Begin graceful shutdown, wait at most num=%d seconds...", h.GetOptions().ExitWaitTimeout/time.Second)
-
-	ctx, cancel := context.WithTimeout(context.Background(), h.GetOptions().ExitWaitTimeout)
-	defer cancel()
-
-	if err := h.Shutdown(ctx); err != nil {
+	if err := h.Shutdown(context.Background()); err != nil {
 		hlog.SystemLogger().Errorf("Shutdown error=%v", err)
 	}
 }
diff --git a/pkg/route/engine.go b/pkg/route/engine.go
index 7c87826..0d1b60c 100644
--- a/pkg/route/engine.go
+++ b/pkg/route/engine.go
@@ -312,9 +312,17 @@ func (engine *Engine) Shutdown(ctx context.Context) (err error) {
 		return
 	}
 
+	opt := engine.GetOptions()
+	hlog.SystemLogger().Infof("Begin graceful shutdown, wait at most %s ...", opt.ExitWaitTimeout)
+
+	ctx, cancel := context.WithTimeout(ctx, opt.ExitWaitTimeout)
+	defer cancel()
+
 	ch := make(chan struct{})
-	// trigger hooks if any
-	go engine.executeOnShutdownHooks(ctx, ch)
+	go func() {
+		defer close(ch)
+		engine.executeOnShutdownHooks(ctx)
+	}()
 
 	defer func() {
 		// ensure that the hook is executed until wait timeout or finish
@@ -328,7 +336,7 @@ func (engine *Engine) Shutdown(ctx context.Context) (err error) {
 		}
 	}()
 
-	if opt := engine.options; opt != nil && opt.Registry != nil {
+	if opt.Registry != nil {
 		if err = opt.Registry.Deregister(opt.RegistryInfo); err != nil {
 			hlog.SystemLogger().Errorf("Deregister error=%v", err)
 			return err
@@ -343,7 +351,7 @@ func (engine *Engine) Shutdown(ctx context.Context) (err error) {
 	return
 }
 
-func (engine *Engine) executeOnShutdownHooks(ctx context.Context, ch chan struct{}) {
+func (engine *Engine) executeOnShutdownHooks(ctx context.Context) {
 	wg := sync.WaitGroup{}
 	for i := range engine.OnShutdown {
 		wg.Add(1)
@@ -353,7 +361,6 @@ func (engine *Engine) executeOnShutdownHooks(ctx context.Context, ch chan struct
 		}(i)
 	}
 	wg.Wait()
-	ch <- struct{}{}
 }
 
 func (engine *Engine) Run() (err error) {
@@ -572,6 +579,7 @@ func (engine *Engine) ServeStream(ctx context.Context, conn network.StreamConn)
 }
 
 func (engine *Engine) initBinderAndValidator(opt *config.Options) {
+	var isCustomValidator bool
 	// init validator
 	if opt.CustomValidator != nil {
 		customValidator, ok := opt.CustomValidator.(binding.StructValidator)
@@ -579,6 +587,7 @@ func (engine *Engine) initBinderAndValidator(opt *config.Options) {
 			panic("customized validator does not implement binding.StructValidator")
 		}
 		engine.validator = customValidator
+		isCustomValidator = true
 	} else {
 		engine.validator = binding.NewValidator(binding.NewValidateConfig())
 		if opt.ValidateConfig != nil {
@@ -587,6 +596,7 @@ func (engine *Engine) initBinderAndValidator(opt *config.Options) {
 				panic("opt.ValidateConfig is not the '*binding.ValidateConfig' type")
 			}
 			engine.validator = binding.NewValidator(vConf)
+			isCustomValidator = true
 		}
 	}
 
@@ -607,7 +617,8 @@ func (engine *Engine) initBinderAndValidator(opt *config.Options) {
 		if !ok {
 			panic("opt.BindConfig is not the '*binding.BindConfig' type")
 		}
-		if bConf.Validator == nil {
+		// optimize: user customized validator has the highest priority
+		if isCustomValidator || bConf.Validator == nil {
 			bConf.Validator = engine.validator
 		}
 		engine.binder = binding.NewDefaultBinder(bConf)


Test patch:
diff --git a/pkg/route/engine_test.go b/pkg/route/engine_test.go
index 2c8c97f..5a5d1a2 100644
--- a/pkg/route/engine_test.go
+++ b/pkg/route/engine_test.go
@@ -801,6 +801,37 @@ func TestValidateConfigSetErrorFactory(t *testing.T) {
 	performRequest(e, "GET", "/bind?b=1")
 }
 
+func TestValidateConfigSetErrorFactoryWithBindConfig(t *testing.T) {
+	type TestValidate struct {
+		B int `query:"b" vd:"$>100"`
+	}
+	opt := config.NewOptions([]config.Option{})
+	CustomValidateErrFunc := func(failField, msg string) error {
+		err := ValidateError{
+			ErrType:   "validateErr",
+			FailField: "[validateFailField]: " + failField,
+			Msg:       "[validateErrMsg]: " + msg,
+		}
+
+		return &err
+	}
+
+	validateConfig := binding.NewValidateConfig()
+	validateConfig.SetValidatorErrorFactory(CustomValidateErrFunc)
+	opt.ValidateConfig = validateConfig
+	opt.BindConfig = binding.NewBindConfig()
+	e := NewEngine(opt)
+	e.GET("/bind", func(c context.Context, ctx *app.RequestContext) {
+		var req TestValidate
+		err := ctx.BindAndValidate(&req)
+		if err == nil {
+			t.Fatal("expect an error")
+		}
+		assert.DeepEqual(t, "validateErr: expr_path=[validateFailField]: B, cause=[validateErrMsg]: ", err.Error())
+	})
+	performRequest(e, "GET", "/bind?b=1")
+}
+
 func TestCustomBinder(t *testing.T) {
 	type Req struct {
 		A int `query:"a"`
@@ -889,12 +920,19 @@ func newMockStandardTransporter(opt *config.Options) network.Transporter {
 }
 
 func TestEngineShutdown(t *testing.T) {
-	mockCtxCallback := func(ctx context.Context) {}
-
 	opt := config.NewOptions(nil)
 	opt.Addr = "127.0.0.1:0"
 	opt.TransporterNewer = newMockStandardTransporter
 
+	mockCtxCallback := func(ctx context.Context) {
+		// Shutdown adds `ExitWaitTimeout` to the given context
+		dl, ok := ctx.Deadline()
+		assert.Assert(t, ok)
+		assert.Assert(t,
+			opt.ExitWaitTimeout-time.Until(dl) < 50*time.Millisecond, // runtime schedule latency
+			opt.ExitWaitTimeout, time.Until(dl))
+	}
+
 	var wg sync.WaitGroup
 	var engine *Engine