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

[FEATURE] - no mechanism to handle panic in jobs for v2 #732

Closed
trungdlp-wolffun opened this issue May 27, 2024 · 10 comments · Fixed by #733
Closed

[FEATURE] - no mechanism to handle panic in jobs for v2 #732

trungdlp-wolffun opened this issue May 27, 2024 · 10 comments · Fixed by #733
Labels
enhancement New feature or request

Comments

@trungdlp-wolffun
Copy link
Contributor

Is your feature request related to a problem? Please describe

package main

import (
	"fmt"
	"time"

	"github.com/go-co-op/gocron/v2"
)

func main() {
	// create a scheduler
	s, err := gocron.NewScheduler()
	if err != nil {
		// handle error
	}

	// add a job to the scheduler
	j, err := s.NewJob(
		gocron.DurationJob(
			10*time.Second,
		),
		gocron.NewTask(
			func(a string, b int) {
				panic("panic occurred")
			},
			"hello",
			1,
		),
	)
	if err != nil {
		// handle error
	}
	// each job has a unique id
	fmt.Println(j.ID())

	// start the scheduler
	s.Start()

	// block until you are ready to shut down
	select {
	case <-time.After(time.Minute):
	}

	// when you're done, shut it down
	err = s.Shutdown()
	if err != nil {
		// handle error
	}
}

Describe the solution you'd like

Describe alternatives you've considered

Additional context

Related issue: #332

@trungdlp-wolffun trungdlp-wolffun added the enhancement New feature or request label May 27, 2024
@khall-ms
Copy link

Have you looked at https://pkg.go.dev/github.com/go-co-op/gocron/v2#example-AfterJobRunsWithError

@khall-ms
Copy link

khall-ms commented Jun 17, 2024

An example would look like:

package main

import (
	"fmt"
	"time"

        "github.com/google/uuid"
	"github.com/go-co-op/gocron/v2"
)

func main() {
	// create a scheduler
	s, err := gocron.NewScheduler()
	if err != nil {
		// handle error
	}

	// add a job to the scheduler
	j, err := s.NewJob(
		gocron.DurationJob(
			10*time.Second,
		),
		gocron.NewTask(
			func(a string, b int) {
				panic("panic occurred")
			},
			"hello",
			1,
		),
                gocron.WithEventListeners(
		    gocron.AfterJobRunsWithError(
			    func(jobID uuid.UUID, jobName string, err error) {
				// do something when the job returns an error
                    fmt.Printf("%c+", err)
			    },
		    ),
              ),
	)
	if err != nil {
		// handle error
	}
	// each job has a unique id
	fmt.Println(j.ID())

	// start the scheduler
	s.Start()

	// block until you are ready to shut down
	select {
	case <-time.After(time.Minute):
	}

	// when you're done, shut it down
	err = s.Shutdown()
	if err != nil {
		// handle error
	}
}

@trungdlp-wolffun
Copy link
Contributor Author

@khall-ms, I think it's not working. I tested with your code, and when a panic occurred, the main program also exited.

f895a01f-27dd-40c0-be06-03e430b116d3
panic: panic occurred

... stack trace

Process finished with the exit code 2

@trungdlp-wolffun
Copy link
Contributor Author

Currently I must recover manual

gocron.NewTask(
  func(a string, b int) {
    defer func() {
	    if r := recover(); r != nil {
	       fmt.Println("recovered from panic")
	    }
    }()
    
    
    panic("panic occurred")
  },
  "hello",
  1,
),
		

@trungdlp-wolffun
Copy link
Contributor Author

trungdlp-wolffun commented Jun 17, 2024

@JohnRoesler please take a look 🥹 and can you guide me on how to fix that, like a senior developer helping a junior developer?

@khall-ms
Copy link

Hey mate,

I think its been a while since I used this library. I'm getting the same error, so either somethings changed or I could have been confusing it with go pond.

I don't have time to go down the rabbit hole atm so you could use this:

package main

import (
    "fmt"
    "time"

    "github.com/google/uuid"
    "github.com/go-co-op/gocron/v2"
)

func main() {
    // create a scheduler
    s, err := gocron.NewScheduler()
    if err != nil {
        // handle error
    }

    // add a job to the scheduler
    j, err := s.NewJob(
        gocron.DurationJob(
            5*time.Second,
        ),
        gocron.NewTask(
            func(a string, b int) (err error) {
                defer func() {
                    if r := recover(); r != nil {
                        err = fmt.Errorf("Recovered. Error:%v", r)
                    }
                }()
                if err != nil {
                    return err
                }

                // Your code here
                panic("panic occurred")

            },
            "hello",
            1,
        ),
        gocron.WithEventListeners(
            gocron.AfterJobRunsWithError(
                func(jobID uuid.UUID, jobName string, err error) {
                // do something when the job returns an error
                    fmt.Printf("Job %s had error %s\n", jobID, err)
                },
            ),
        ),
    )
    if err != nil {
        // handle error
    }
    // each job has a unique id
    fmt.Println(j.ID())

    // start the scheduler
    s.Start()

    // block until you are ready to shut down
    select {
    case <-time.After(time.Minute):
    }

    // when you're done, shut it down
    err = s.Shutdown()                                                                                                                                                                       if err != nil {
        // handle error
    }
}

@trungdlp-wolffun
Copy link
Contributor Author

I think we should add a panic handler like this: #335 @khall-ms

@khall-ms
Copy link

That would be ideal 💯

@JohnRoesler
Copy link
Contributor

Thanks for the collaborative discussion on this. 🚀 I requested changes on the PR to fit the design with how other "event listener" things are handled.

I'm curious, what type of logic are you running in your jobs that is resulting in panics? I haven't added panic handling support because my general philosophy is that your code should only ever panic during development.

@trungdlp-wolffun
Copy link
Contributor Author

@JohnRoesler, sometimes I deal with nil pointers or division by zero errors

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

Successfully merging a pull request may close this issue.

3 participants