starred/asynq

Fork 0

mirror of https://github.com/hibiken/asynq.git synced 2026-04-25 23:15:51 +03:00

[GH-ISSUE #313] [FEATURE REQUEST] Change process task context behaviour #1147

New issue

Closed

opened 2026-03-07 22:06:27 +03:00 by kerem · 4 comments

kerem commented

2026-03-07 22:06:27 +03:00

Owner

Originally created by @crossworth on GitHub (Aug 19, 2021).
Original GitHub issue: https://github.com/hibiken/asynq/issues/313

Originally assigned to: @hibiken on GitHub.

Is your feature request related to a problem? Please describe.
Maybe. It could help solve #285 and #265 and make the library more idiomatic.

Describe the solution you'd like
I would like to propose that we change the ProcessTask signature from:

ProcessTask(ctx context.Context, t *asynq.Task) error

To:

ProcessTask(t *asynq.Task) error

And that we add the context.Context to the task struct to make it more open to middleware patterns.

// Task represents a unit of work to be performed.
type Task struct {
	// typename indicates the type of task to be performed.
	typename string

	// payload holds data needed to perform the task.
	payload []byte

	// context holds the task context.
	context context.Context
}
// ...
func (t *Task) Context() context.Context { return t.context }

// WithContext returns a deep copy of the Task with its context changed
// to ctx. The provided ctx must be non-nil.
// The context is not stored when enqueuing a task.
func (t *Task) WithContext(ctx context.Context) *Task {
	if ctx == nil {
		panic("nil context")
	}

	newTask := &Task{
		typename: t.typename,
		payload:  make([]byte, len(t.payload)),
		context:  ctx,
	}

	copy(newTask.payload, t.payload)

	return newTask
}

// ...

// newTaskWithContext returns a new Task given a context, type name and payload data.
// The provided ctx must be non-nil.
func newTaskWithContext(ctx context.Context, typename string, payload []byte) *Task {
	if ctx == nil {
		panic("nil context")
	}

	return &Task{
		typename: typename,
		payload:  payload,
		context:  ctx,
	}
}

Describe alternatives you've considered
None, but as stated in https://github.com/hibiken/asynq/issues/285#issuecomment-870732712 we could use a database to hold state between the middleware and task, this would solve some of the problems that the missing context causes.

Additional context
The current implemenation allows many of the middleware patterns, like allowing a middleware setting a value on context, metrics, tracing, but for more complex middleware scenarios is not perfect. Like in the case where we want to use the context to pass information back to middleware. Placing the context of the task we make more clear that the context is only valid for that task as well, and ofcourse this make the task more like thehttp.Request and all the patterns that are common on http middleware/handlers today.

It's important to note that it may cause confusion on task registration, the consumer may think that it can store context value and it will be serialized and stored on the storage engine, for this reason I think we should limit the context API, allowing only the context retrieve and WithContext that creates a copy of a task (to be used inside middleware or task handlers).

I have implemented this propose on https://github.com/crossworth/asynq/tree/feature/change-process-signature and have create a repository with examples using this propose https://github.com/crossworth/asynq-task-ctx-examples

The examples includes:

pass value from middleware to task
return value from task to middleware
tracing
metrics
logging

What do you think @hibiken? If you think this is a good improvement let me know that I will create the PR and test with asynqmon/cli (currently tests are passing).

Originally created by @crossworth on GitHub (Aug 19, 2021). Original GitHub issue: https://github.com/hibiken/asynq/issues/313 Originally assigned to: @hibiken on GitHub. **Is your feature request related to a problem? Please describe.** Maybe. It could help solve #285 and #265 and make the library more idiomatic. **Describe the solution you'd like** I would like to propose that we change the ProcessTask signature from: ```go ProcessTask(ctx context.Context, t *asynq.Task) error ``` To: ```go ProcessTask(t *asynq.Task) error ``` And that we add the `context.Context` to the task struct to make it more open to middleware patterns. ```go // Task represents a unit of work to be performed. type Task struct { // typename indicates the type of task to be performed. typename string // payload holds data needed to perform the task. payload []byte // context holds the task context. context context.Context } // ... func (t *Task) Context() context.Context { return t.context } // WithContext returns a deep copy of the Task with its context changed // to ctx. The provided ctx must be non-nil. // The context is not stored when enqueuing a task. func (t *Task) WithContext(ctx context.Context) *Task { if ctx == nil { panic("nil context") } newTask := &Task{ typename: t.typename, payload: make([]byte, len(t.payload)), context: ctx, } copy(newTask.payload, t.payload) return newTask } // ... // newTaskWithContext returns a new Task given a context, type name and payload data. // The provided ctx must be non-nil. func newTaskWithContext(ctx context.Context, typename string, payload []byte) *Task { if ctx == nil { panic("nil context") } return &Task{ typename: typename, payload: payload, context: ctx, } } ``` **Describe alternatives you've considered** None, but as stated in https://github.com/hibiken/asynq/issues/285#issuecomment-870732712 we could use a database to hold state between the middleware and task, this would solve some of the problems that the missing context causes. **Additional context** The current implemenation allows many of the middleware patterns, like allowing a middleware setting a value on context, metrics, tracing, but for more complex middleware scenarios is not perfect. Like in the case where we want to use the context to pass information back to middleware. Placing the context of the task we make more clear that the context is only valid for that task as well, and ofcourse this make the task more like the`http.Request` and all the patterns that are common on http middleware/handlers today. It's important to note that it may cause confusion on task registration, the consumer may think that it can store context value and it will be serialized and stored on the storage engine, for this reason I think we should limit the context API, allowing only the context retrieve and `WithContext` that creates a copy of a task (to be used inside middleware or task handlers). I have implemented this propose on https://github.com/crossworth/asynq/tree/feature/change-process-signature and have create a repository with examples using this propose https://github.com/crossworth/asynq-task-ctx-examples The examples includes: - pass value from middleware to task - return value from task to middleware - tracing - metrics - logging What do you think @hibiken? If you think this is a good improvement let me know that I will create the PR and test with asynqmon/cli (currently tests are passing).

kerem

2026-03-07 22:06:27 +03:00

closed this issue
added the
enhancement
label

kerem commented

2026-03-07 22:06:28 +03:00

Author

Owner

@hibiken commented on GitHub (Aug 21, 2021):

@crossworth Thank you for the proposal!
I think there's a huge benefit in making the API similar to net/http package, since there are lot of other packages built around the net/http and makes this package integrate with those packages easier.

Since this is a breaking change, I'd like to be careful when making the decision. Let's list out pros and cons of the current and suggested API and we can go from there!

@hibiken commented on GitHub (Aug 21, 2021): @crossworth Thank you for the proposal! I think there's a huge benefit in making the API similar to `net/http` package, since there are lot of other packages built around the `net/http` and makes this package integrate with those packages easier. Since this is a breaking change, I'd like to be careful when making the decision. Let's list out pros and cons of the current and suggested API and we can go from there!

kerem commented

2026-03-07 22:06:28 +03:00

Author

Owner

@crossworth commented on GitHub (Aug 22, 2021):

I will work on enumerating the pros and cons.

Just a note, we could solve this problem by changing the function signature from func(ctx context.Context, t *asynq.Task) error to func(ctx context.Context, t *asynq.Task) (context.Context, error) as well, but I think we should try mimic the net/http way of handling requests and since we are on the v0 its easy to do it now than later.

@crossworth commented on GitHub (Aug 22, 2021): I will work on enumerating the pros and cons. Just a note, we could solve this problem by changing the function signature from `func(ctx context.Context, t *asynq.Task) error` to `func(ctx context.Context, t *asynq.Task) (context.Context, error)` as well, but I think we should try mimic the `net/http` way of handling requests and since we are on the v0 its easy to do it now than later.

kerem commented

2026-03-07 22:06:28 +03:00

Author

Owner

@crossworth commented on GitHub (Aug 28, 2021):

Current API
PROS:

It's already implemented and works for almost all cases.

CONS:

The context is handled in a unilateral way, we receive the context, but we cannot change it, this limit the cases where a middleware is a solution (or complete solution), like returning custom data or adding contextual data to be used by a middleware.
The API is more like the database apis that accept the context as the first param than the http handlers API, I think the handling a task should be analog to http handlers.

Suggested API
PROS:

It's more similar to http handlers and by doing so is more idiomatic, this can be good, since with little modification a http middleware could be used on a task middleware.
Allows more control to the task handler, right now a task handler can only return sentinel errors to control the task execution, we can use context as a generic container e.g: change the task execution (returning sentinel errors) on middleware based on context values.

CONS:

It's a breaking change
The user could think that the context is serialized and saved/retrived when processing atask (by using WithContext when creating the task), we should document the behavior (ideally it would be cool to serialize and store the context as well, but I dont know if is possible or if we should).

The context may be used on the workflow feature (#244) (like sharing the same context between tasks) as well.

I know that the pros and cons are not that strong, but allowing the task to change the context and the middleware consume it is a big benefit.

@crossworth commented on GitHub (Aug 28, 2021): **Current API** PROS: - It's already implemented and works for almost all cases. CONS: - The context is handled in a unilateral way, we receive the context, but we cannot change it, this limit the cases where a middleware is a solution (or complete solution), like returning custom data or adding contextual data to be used by a middleware. - The API is more like the database apis that accept the context as the first param than the http handlers API, I think the handling a task should be analog to http handlers. **Suggested API** PROS: - It's more similar to http handlers and by doing so is more idiomatic, this can be good, since with little modification a http middleware could be used on a task middleware. - Allows more control to the task handler, right now a task handler can only return sentinel errors to control the task execution, we can use context as a generic container e.g: change the task execution (returning sentinel errors) on middleware based on context values. CONS: - It's a breaking change - The user could think that the context is serialized and saved/retrived when processing atask (by using WithContext when creating the task), we should document the behavior (ideally it would be cool to serialize and store the context as well, but I dont know if is possible or if we should). The context may be used on the workflow feature (#244) (like sharing the same context between tasks) as well. I know that the pros and cons are not that strong, but allowing the task to change the context and the middleware consume it is a big benefit.

kerem commented

2026-03-07 22:06:29 +03:00

Author

Owner

@hibiken commented on GitHub (Sep 7, 2021):

@crossworth thank you for putting this together!

I'm currently designing the implementation of workflow and will take this proposal into consideration 👍

@hibiken commented on GitHub (Sep 7, 2021): @crossworth thank you for putting this together! I'm currently designing the implementation of [workflow](https://github.com/hibiken/asynq/issues/244) and will take this proposal into consideration 👍

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

starred/asynq#1147

No description provided.

Rows
Columns