Skip to content
This repository has been archived by the owner on Sep 20, 2022. It is now read-only.

Error Handling? #39

Open
pyrossh opened this issue Mar 3, 2016 · 5 comments
Open

Error Handling? #39

pyrossh opened this issue Mar 3, 2016 · 5 comments

Comments

@pyrossh
Copy link

pyrossh commented Mar 3, 2016

Is it possible to add a common error handler for all jobs that return an error so that I can say log my errors during development and send mails during production. Because right now in development I just return the error and don't know when and where the job failed.

@pyrossh
Copy link
Author

pyrossh commented Mar 3, 2016

I tried this but this doesn't return the error that caused the job to fail

jobPool.SetAfterFunc(func(j *jobs.Job) {
  err := j.Error()
})

@albrow
Copy link
Owner

albrow commented Mar 7, 2016

@pyros2097 thanks for bringing this up.

The documentation for SetAfterFunc states:

SetAfterFunc will assign a function that will be executed each time a job is finished.

This means that the function will not be executed if there is an error. I'm sorry if that is not clear. Do you think we could improve the wording?

Secondly, I'm not sure if this use case warrants the addition of a new feature. It seems like you should be able to write your own error handler function and then call it whenever there is an error. Something like this:

func handleErr(j *jobs.Job, err error) {
    // Log the error and do whatever else you need to do here. 
}

// When registering the job, we can handle the error manually.
welcomeEmailJobs, err := jobs.RegisterType("welcomeEmail", 3, func(user *User) error {
    msg := fmt.Sprintf("Hello, %s! Thanks for signing up for foo.com.", user.Name)
    if err := emails.Send(user.EmailAddress, msg); err != nil {
        // Before returning the erorr, call handleErr
        handleErr(err)
        return err
    }
})

You know more about your code than I do. Do you think this would work? Or do you think we need a new feature for this?

@pyrossh
Copy link
Author

pyrossh commented Mar 8, 2016

@albrow Yeah I guess the SetAfterFunc's doc is a bit misleading in a way that it executes after a job is finished (whether success or failure) and the job also contains an error object (since a successful job shouldn't contain an error) on it so I naturally thought thats how its done.

Anyway some of our jobs are very large and there are ton's of errors happening in the process. And since your RegisterType func already expects an error to be returned whenever some thing goes wrong in the handler it can send this error and the job to a default error handler which will notify us of the error and provide us more context on it and not just an error.

We use echo router and it has a really neat error handling mechanism . See echo router's default error handling mechanism DefaultHTTPErrorHandler. We get the error and even a context object on where it happend and this could help us more in fixing bugs in production.
And maybe a panic handler for the job will also be useful.

We could do the handleErr(err) func like you said but I was thinking this would make it more easier from a coding perspective and maybe we could also save these failed jobs somewhere (maybe postgres) so that we could retry them or list them later.

@pyrossh
Copy link
Author

pyrossh commented Mar 9, 2016

We also need to have a common panic handler for the jobs since recently one of the jobs panic'ked and killed our server. So for now I'm manually doing a defer HandlePanic() for it just to be safe in each job.

edit: Oh wait your's does have a panic handler this happened because of this https://github.com/carlescere/scheduler forgot to switch this in the other branch

@pyrossh
Copy link
Author

pyrossh commented Mar 18, 2016

Just a heads up,
Right now I do my error handling like this

catchError := func(process func() (*Context, error)) error {
        defer HandlePanic()
        context, err := process()
        if err != nil {
            MakeStackTrace(context, err)
        }
        logger.Warn("Job Error", "err", err)
        return err
}
jobs.RegisterType("email", 3, func(params *Message) error {
            return catchError(func() (*Context, error) {
            } 
}

This does make it easier a liitle bit.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants