feat: add OnStart hook for synchronous startup jobs #2079

HeerakKashyap · 2025-07-19T17:20:13Z

Description:

Implements an OnStart hook for the GoFr framework, allowing users to register synchronous startup jobs that execute before the server starts.
The hook receives a fully initialized *gofr.Context with all DI-managed services, as requested by maintainers.
Removes any previous custom context logic and follows the GoFr way for dependency injection.
Updates documentation and provides usage examples.
Includes proof of working feature (see attached screenshots).

Testing:

Verified that OnStart hooks can access DI-managed services (e.g., SQL, Redis).
All relevant tests pass with MySQL and Redis running in Docker.

Fixes #1833

Let me know if you want to add or change anything, or if you need help attaching screenshots or filling out the checklist!

…ode quality

ccoVeille · 2025-07-19T17:59:07Z

go.work.sum

I'm unsure why this was changed. As if code using a different logger was added

I was building a new docker container, it automatically changes if u run so "go build " kinds of prompts

@HeerakKashyap Please reomve these changes. There should be no changes to go.work.sum file.

pkg/gofr/run.go

Umang01-hash

There are no tests added for the newly added code.
No documentation updates for the new feature.
We have following code quality/linter issues in your PR:

Please resolve them along with other review comments.
Thankyou

pkg/gofr/run.go

examples/http-server/main.go

…t DI access

HeerakKashyap · 2025-07-21T13:35:36Z

There are no tests added for the newly added code.

No documentation updates for the new feature.

We have following code quality/linter issues in your PR:

Please resolve them along with other review comments. Thankyou

should i update iin readme.md , it would be okay to do so right ? @Umang01-hash

Umang01-hash · 2025-07-21T13:43:42Z

There are no tests added for the newly added code.

No documentation updates for the new feature.

We have following code quality/linter issues in your PR:

Please resolve them along with other review comments. Thankyou

should i update iin readme.md , it would be okay to do so right ? @Umang01-hash

@HeerakKashyap I think we can update the existing docs or maybe create a new one for the same. Both can work depending upon the content we are sharing.

- Move OnStart logic to a dedicated, testable method. - Fix all linter issues (deep-exit and cyclomatic complexity). - Add unit tests for success and error cases. - Add comprehensive documentation with a realistic example. - Update the example app to demonstrate a real-world use case.

HeerakKashyap · 2025-07-21T14:41:06Z

Test case are working fine @Umang01-hash @ccoVeille

pkg/gofr/run.go

…own on context cancel)

…hyap/gofr into fix/onstart-di-context

HeerakKashyap · 2025-07-22T18:29:48Z

kidnly have a look @ccoVeille @Umang01-hash

pkg/gofr/run.go

ccoVeille · 2025-07-22T19:12:51Z

The code is in, but I would prefer if the context was not forged twice.

Can't we create the gofr.Context before calling startup hooks, then pass it through all the layers as argument up to the place where the gofr.Context is created now?

Or would this implies breaking changes,? Or simply too much changes?

Please note, it doesn't necessary have to be addressed in this PR.

HeerakKashyap · 2025-07-23T04:57:22Z

The code is in, but I would prefer if the context was not forged twice.

Can't we create the gofr.Context before calling startup hooks, then pass it through all the layers as argument up to the place where the gofr.Context is created now?

Or would this implies breaking changes,? Or simply too much changes?

Please note, it doesn't necessary have to be addressed in this PR.

Thank you for the suggestion Sir! @ccoVeille

I agree that creating the gofr.Context only once and passing it through would be cleaner and more efficient.
However, implementing this would require significant refactoring and could potentially introduce breaking changes, especially if other parts of the framework expect to construct their own context.

For this PR, I’ll keep the current approach to avoid introducing breaking changes, but I think this is a great idea for future improvement and can be considered in a dedicated refactor.

HeerakKashyap · 2025-07-23T05:08:39Z

@Umang01-hash sir, kindly run the workflows and let me know is there now any linter errors etc.

pkg/gofr/gofr.go

Umang01-hash · 2025-07-23T07:27:38Z

Hey @HeerakKashyap only a few things:

Changes in go.work.sum file needs to be reverted.
We dont need a newContextForHooks methods, we can use exitisting methods to create new context.
A lot of linter errors are there in the code we need to resolve them.

Please do the above changes and we should be ready to go with your PR.

- Fix dynamic error definition - Fix unused parameters with underscore - Use t.Context() instead of context.Background() - Use assert.NoError instead of assert.Nil

- Fix cuddled return statement in newContextForHooks - Fix cuddled assignment in OnStart test - Remove os.Exit call to resolve deep-exit error

…ction

HeerakKashyap · 2025-07-23T18:01:59Z

@Umang01-hash kidnly , check the changes out

Umang01-hash · 2025-07-24T05:23:58Z

Screenshot 2025-07-24 at @HeerakKashyap 0 53 08 AM

Hey @HeerakKashyap. Please resolve the linter issues on your PR.

…tainability - Fix err113: Replace dynamic error creation with static error in TestApp_OnStart - Fix gocyclo: Refactor Run() method by extracting helper methods to reduce cyclomatic complexity - Fix ws1: Correct whitespace issues in gofr_test.go and run.go - Add helper methods: handleStartupHooks, startShutdownHandler, startTelemetryIfEnabled, startAllServers - Improve code organization and readability while maintaining functionality

HeerakKashyap · 2025-07-24T07:19:45Z

Hey @HeerakKashyap. Please resolve the linter issues on your PR.

i had to make couple of changes to fix the complexity issue , kindly see if this works @Umang01-hash

- Fix err113: Move error variable to package level to avoid dynamic error creation - Fix error-naming: Rename hookFailedErr to errHookFailed following Go naming conventions - Fix whitespace: Remove unnecessary blank line before app assignment - All linting issues in modified files are now resolved

HeerakKashyap · 2025-07-24T11:51:08Z

@Umang01-hash So this PR is ready to get merged ?

go.work.sum

pkg/gofr/run.go

HeerakKashyap · 2025-07-24T19:55:53Z

@Umang01-hash CAN U LET me know how can i run linter test locally, if there's some application in Golang for running linter tests

Umang01-hash · 2025-07-25T06:10:19Z

@Umang01-hash CAN U LET me know how can i run linter test locally, if there's some application in Golang for running linter tests

Hey @HeerakKashyap! You can follow this command to install linter :

go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1

then export it's path and you can run the linters locally using golangci-lint run command inside the GoFr directory.

HeerakKashyap · 2025-07-25T06:22:37Z

@Umang01-hash CAN U LET me know how can i run linter test locally, if there's some application in Golang for running linter tests

Hey @HeerakKashyap! You can follow this command to install linter :
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1 
then export it's path and you can run the linters locally using golangci-lint run command inside the GoFr directory.

okay Sir, let me do this then i'll let u know
Thanks!

HeerakKashyap · 2025-07-25T06:32:20Z

@Umang01-hash Code quality tests are passed, what's next ?

HeerakKashyap · 2025-07-25T06:38:18Z

@Umang01-hash CAN U LET me know how can i run linter test locally, if there's some application in Golang for running linter tests

Hey @HeerakKashyap! You can follow this command to install linter :
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1 
then export it's path and you can run the linters locally using golangci-lint run command inside the GoFr directory.

all linter issues are solved, I tested it locally
@Umang01-hash

HeerakKashyap · 2025-07-25T15:00:06Z

@Umang01-hash . KINDLY REVIEW

feat: improve OnStart hook with DI context, update docs, and ensure c…

8202ed1

…ode quality

ccoVeille reviewed Jul 19, 2025

View reviewed changes

pkg/gofr/run.go Outdated Show resolved Hide resolved

Umang01-hash requested changes Jul 21, 2025

View reviewed changes

pkg/gofr/run.go Outdated Show resolved Hide resolved

pkg/gofr/run.go Outdated Show resolved Hide resolved

pkg/gofr/run.go Outdated Show resolved Hide resolved

examples/http-server/main.go Outdated Show resolved Hide resolved

refactor: improve OnStart hook context, move logic to method, and tes…

0cb726b

…t DI access

HeerakKashyap force-pushed the fix/onstart-di-context branch from 57c4743 to 8526959 Compare July 21, 2025 14:38

Merge branch 'development' into fix/onstart-di-context

2cbe7e1

HeerakKashyap changed the title ~~feat: add OnStart hook for synchronous startup jobs (closes #1833)~~ feat: add OnStart hook for synchronous startup jobs Jul 22, 2025

Umang01-hash and others added 2 commits July 22, 2025 14:15

Merge branch 'development' into fix/onstart-di-context

79f0fa5

Merge branch 'development' into fix/onstart-di-context

2cf9cbf

ccoVeille reviewed Jul 22, 2025

View reviewed changes

pkg/gofr/run.go Outdated Show resolved Hide resolved

HeerakKashyap added 2 commits July 22, 2025 23:54

fix: improve startup error handling for OnStart hooks (graceful shutd…

9fd31d4

…own on context cancel)

Merge branch 'fix/onstart-di-context' of https://github.com/HeerakKas…

7c38332

…hyap/gofr into fix/onstart-di-context

ccoVeille reviewed Jul 22, 2025

View reviewed changes

pkg/gofr/run.go Outdated Show resolved Hide resolved

ccoVeille approved these changes Jul 23, 2025

View reviewed changes

Umang01-hash requested changes Jul 23, 2025

View reviewed changes

pkg/gofr/gofr.go Outdated Show resolved Hide resolved

pkg/gofr/gofr.go Outdated Show resolved Hide resolved

HeerakKashyap added 3 commits July 23, 2025 22:56

fix: address linter errors in OnStart tests

27ca38f

- Fix dynamic error definition - Fix unused parameters with underscore - Use t.Context() instead of context.Background() - Use assert.NoError instead of assert.Nil

fix: remove os.Exit call to resolve deep-exit linter error

463b194

fix: resolve remaining linter errors

9e8bcdf

- Fix cuddled return statement in newContextForHooks - Fix cuddled assignment in OnStart test - Remove os.Exit call to resolve deep-exit error

HeerakKashyap force-pushed the fix/onstart-di-context branch from f4882d6 to 9e8bcdf Compare July 23, 2025 17:52

refactor: remove newContextForHooks and reuse existing newContext fun…

55d94eb

…ction

HeerakKashyap added 2 commits July 24, 2025 13:12

fix: resolve godot and wsl linter errors in run.go and gofr_test.go

60b6ca9

HeerakKashyap requested a review from Umang01-hash July 24, 2025 08:42

Umang01-hash requested changes Jul 24, 2025

View reviewed changes

go.work.sum Outdated Show resolved Hide resolved

pkg/gofr/run.go Outdated Show resolved Hide resolved

pkg/gofr/run.go Show resolved Hide resolved

HeerakKashyap and others added 4 commits July 24, 2025 17:53

Update go.work.sum

6b9639b

fix: restore comments as requested by reviewer

8c6a4b6

Update run.go

e950953

Update run.go

bada9a7

HeerakKashyap requested a review from Umang01-hash July 24, 2025 13:20

Umang01-hash and others added 2 commits July 25, 2025 11:40

Merge branch 'development' into fix/onstart-di-context

9f2b697

merge: resolve conflicts and fix formatting

2767521

Umang01-hash and others added 2 commits July 25, 2025 17:30

Merge branch 'development' into fix/onstart-di-context

eddd360

Merge branch 'development' into fix/onstart-di-context

d21b836

feat: add OnStart hook for synchronous startup jobs #2079

Are you sure you want to change the base?

feat: add OnStart hook for synchronous startup jobs #2079

Uh oh!

Conversation

HeerakKashyap commented Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccoVeille Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

HeerakKashyap Jul 19, 2025

Choose a reason for hiding this comment

Uh oh!

Umang01-hash Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Umang01-hash left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HeerakKashyap commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Umang01-hash commented Jul 21, 2025

Uh oh!

HeerakKashyap commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

HeerakKashyap commented Jul 22, 2025

Uh oh!

Uh oh!

ccoVeille commented Jul 22, 2025

Uh oh!

HeerakKashyap commented Jul 23, 2025

Uh oh!

HeerakKashyap commented Jul 23, 2025

Uh oh!

Uh oh!

Uh oh!

Umang01-hash commented Jul 23, 2025

Uh oh!

HeerakKashyap commented Jul 23, 2025

Uh oh!

Umang01-hash commented Jul 24, 2025

Uh oh!

HeerakKashyap commented Jul 24, 2025

Uh oh!

HeerakKashyap commented Jul 24, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HeerakKashyap commented Jul 24, 2025

Uh oh!

Umang01-hash commented Jul 25, 2025

Uh oh!

HeerakKashyap commented Jul 25, 2025

Uh oh!

HeerakKashyap commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HeerakKashyap commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HeerakKashyap commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

HeerakKashyap commented Jul 19, 2025 •

edited

Loading

Umang01-hash left a comment •

edited

Loading

HeerakKashyap commented Jul 21, 2025 •

edited

Loading

HeerakKashyap commented Jul 21, 2025 •

edited

Loading

HeerakKashyap commented Jul 25, 2025 •

edited

Loading

HeerakKashyap commented Jul 25, 2025 •

edited

Loading

HeerakKashyap commented Jul 25, 2025 •

edited

Loading