I created the following code to help remove some boilerplate around go routines/channels and also make it somewhat more similar to how other languages use async/await
a few questions:
is this a bad practice in go? why?
if its ok, is there anything I should do different?
is there a more complete solution available for this (I checked pkgs but I’m not sure if my pkg search foo is there yet)
package task
import (
"context"
"fmt"
)
type TaskStatus int
const (
New TaskStatus = iota
Running
Completed
)
var taskStatus = [3]string{"New", "Running", "Completed"}
type taskFunc[T interface{}] func(ctx context.Context) (T, error)
type voidTaskFunc func(ctx context.Context) error
type Task[T interface{}] interface {
Status() TaskStatus
Result() (T, error)
Await(ctx ...context.Context) (T, error)
}
type task[T interface{}] struct {
result T
err error
c chan struct{}
status TaskStatus
fn taskFunc[T]
}
type TaskPanicError struct {
reason interface{}
}
func (e TaskPanicError) Error() string {
return fmt.Sprintf("Task panicked: %v", e.reason)
}
func NewTask[T interface{}](fn taskFunc[T]) *task[T] {
t := new(task[T])
t.c = make(chan struct{})
t.fn = fn
return t
}
func (t *task[T]) Status() TaskStatus {
return t.status
}
func (t *task[T]) Result() (T, error) {
switch t.status {
case Completed:
return t.result, t.err
default:
return t.result, fmt.Errorf("Task status is %s", taskStatus[t.status])
}
}
func (t *task[T]) Await(ctx ...context.Context) (T, error) {
switch t.status {
case Completed:
return t.result, t.err
case Running:
return t.result, fmt.Errorf("Task already running, can only be started once")
}
defer func() {
t.status = Completed
}()
var _ctx context.Context
if len(ctx) == 0 {
_ctx = context.Background()
} else {
_ctx = ctx[0]
}
go t.start(_ctx)
select {
case <-_ctx.Done():
return t.result, _ctx.Err()
case <-t.c:
return t.result, t.err
}
}
func (t *task[T]) start(ctx context.Context) {
defer func() {
if r := recover(); r != nil {
t.err = TaskPanicError{reason: r}
}
t.status = Completed
t.c <- struct{}{}
close(t.c)
}()
if ctx.Err() != nil {
t.err = ctx.Err()
t.c <- struct{}{}
return
}
t.status = Running
t.result, t.err = t.fn(ctx)
}
As far as I can see, you try to make a universal interface to run simple functions. It’s a good practice, but imho mostly to learn the algorithm, since it’s a very ideal scenario, when you need to run something like func(ctx context.Context) (any, error). Your example is very good. Nevertheless, I would like to make some advices.
In Await function, use only simple context.Context as argument, let the user to pass background or TODO on their own, less assumptions.
Add stringer to your custom status. It’s a very simple and useful tool, which allows you to lower possible errors with additional slices for string variables.
type TaskStatus int
//go:generate stringer -type=TaskStatus
const (
New TaskStatus = iota
Running
Completed
Error
)
Every time you add, remove, switch places of iota for your status, simple generate command will help you tons of time. This will allow you to simple use t.status with %s in fmt.
I would say having Result and Await return the same result and error is a little bit too much. I would split them into their purposes. Let Await return only task related issues.
I would add some locks on read/write data from your struct, since there are possible concurrent access to different data, which can be in the middle of the change.
The code below is also not ideal. It’s just mho, how I would’ve move in this particular example.
package task
import (
"context"
"fmt"
"sync"
)
type TaskStatus int
//go:generate stringer -type=TaskStatus
const (
New TaskStatus = iota
Running
Completed
Error
)
type taskFunc[T any] func(ctx context.Context) (T, error)
type voidTaskFunc func(ctx context.Context) error
type Task[T any] interface {
Status() TaskStatus
Result() (T, error)
Await(ctx ...context.Context) (T, error)
}
type task[T any] struct {
sync.RWMutex
result T
err error
c chan struct{}
status TaskStatus
fn taskFunc[T]
}
type TaskPanicError struct {
reason any
}
func (e TaskPanicError) Error() string {
return fmt.Sprintf("Task panicked: %v", e.reason)
}
func NewTask[T any](fn taskFunc[T]) *task[T] {
t := new(task[T])
t.c = make(chan struct{})
t.fn = fn
return t
}
func (t *task[T]) Status() TaskStatus {
t.RLock()
defer t.RUnlock()
return t.status
}
func (t *task[T]) Result() (T, error) {
t.RLock()
defer t.RUnlock()
switch t.status {
case Completed:
return t.result, t.err
default:
return t.result, fmt.Errorf("Task status is %s", t.status)
}
}
func (t *task[T]) Await(ctx context.Context) error {
switch t.Status() {
case Completed:
return t.err
case Running:
return fmt.Errorf("Task already running, can only be started once")
}
t.Lock()
defer t.Unlock()
t.status = Running
go t.start(ctx)
select {
case <-ctx.Done():
t.status = Error
return ctx.Err()
case <-t.c:
t.status = Completed
return t.err
}
}
func (t *task[T]) start(ctx context.Context) {
defer func() {
if r := recover(); r != nil {
t.err = TaskPanicError{reason: r}
}
close(t.c)
}()
t.result, t.err = t.fn(ctx)
}
The biggest problem is concurrent access to your variables.
If multiple go-routines access the same variable and at least one of them is writing, you need to synchronize the access. Either by using Atomic accessors, Mutex or channels.
I think the most idiomatic go-way to do it would be using channels. The implementation of Context with timeout is a good example.
Personally I would probably simplify the interface quite a bit. Creating a Task without starting it seems superfluous, it can simply stay a plain function. So one could change the api to only have a single method: StartNewTask(context, func) and a Task only has two states: Running and Finished (with a result, which could be an error)
I usually focus on the utility first and work backwards from there. What does one need the task/await methods for? Usually for things like “Run an array of tasks, max 4 in parallel at a time and return all results as an array when all are done.”
You could try to find these use cases and work backwards from there to find the right structure to fulfill them instead of starting with a structure without clear purpose.