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

Make jobCancelError and jobSnoozeError public #625

Open
jace-ys opened this issue Sep 27, 2024 · 1 comment
Open

Make jobCancelError and jobSnoozeError public #625

jace-ys opened this issue Sep 27, 2024 · 1 comment

Comments

@jace-ys
Copy link

jace-ys commented Sep 27, 2024

Hey 👋🏻 I'm not entirely sure why jobCancelError and jobSnoozeError are not exported as part of the public API, especially because jobCancelError also implements errors.Is and errors.Unwrap.

What I'm trying to achieve is to wrap my actual Work functions with an instrumented Work helper that logs/traces job errors. Right now the only way to check if the actual Work returned river.JobCancel or river.JobSnooze is to do a string prefix match, which isn't ideal.

For example:

	err = w.worker.Work(ctx, job)
	if err != nil {
		var errReason string
		switch {
		case strings.HasPrefix(err.Error(), "jobCancelError:"):
			errReason = "job cancelled"
			span.SetAttributes(attribute.Bool("job.cancelled", true))

		case strings.HasPrefix(err.Error(), "jobSnoozeError:"):
			slog.Info(ctx, "job snoozed")
			span.SetAttributes(attribute.Bool("job.snoozed", true))
			return err

		case job.Attempt == job.MaxAttempts:
			errReason = "job failed, discarded due to max attempts exceeded"
			span.SetAttributes(attribute.Bool("job.discarded", true))

		default:
			errReason = "job failed"
			span.SetAttributes(attribute.Bool("job.failed", true))
		}

		slog.Error(ctx, errReason, err)
		span.SetStatus(codes.Error, errReason)
		span.SetAttributes(attribute.String("error", err.Error()))

		return err
	}

If the error types were made public, I will be able to simplify this using errors.Is(err, &river.JobCancelError{}).

@Pascal-Delange
Copy link

Hi,
I was going to make the same ticket, independently.
This seems like a low-pain change that makes implementing a logging middleware easier.

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

No branches or pull requests

2 participants