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

Consider make RequestFuture more elegent? #772

Open
jidibinlin opened this issue Jan 3, 2023 · 5 comments
Open

Consider make RequestFuture more elegent? #772

jidibinlin opened this issue Jan 3, 2023 · 5 comments

Comments

@jidibinlin
Copy link

jidibinlin commented Jan 3, 2023

Is your feature request related to a problem? Please describe.
I`m using protoactor-go for our project.

But I found We can make RequestFuture more elegant. consider this code snippet

// request
roomPidRes, err := System.Root.RequestFuture(roomMgrPid, info, 1*time.Second).Result()

// respond
context.Respond(&FooRoomPid)

err`s type is only time out error. I think we have a chance to reuse it.
Describe the solution you'd like
We can Define a response struct

type struct Response {
RetValue interface{}
Err error
}

When we use Response struct as a response value. Requester won't wait until timeOut, and when err occurred Requester will know exactly what happened.

@jidibinlin jidibinlin changed the title Consider make RequestFuture more eleg? Consider make RequestFuture more elegent? Jan 3, 2023
@cupen
Copy link
Contributor

cupen commented Jan 4, 2023

> err's type is only time out error.
Nope, some errors defined in remote/errors.go and actor/future.go.

> when err occurred Requester will know exactly what happened.
Sure, I agree with you. We can just use error which is a built-in interface type in Go.

> We can Define a response struct
Do you mean a struct wrapper of Response and Error?
It looks like the Optional in Java or Option in rust.
That's cool, but it's not zero-cost abstract in Go, and doesn't follow the default Go style right now. Maybe Go will make a change in the future or maybe not.

> When we use Response struct as a response value. Requester won't wait until timeOut
Response struct isn't necessary with that case.
For example:
The ErrTimeout is meant that no response in a certain amount of time.
The ErrDeadLetter is meant that you request to a unreachable actor.
...
The ErrBlahblah is meant that blahblah...
...
So we can define a lot errors with different cases.

@jidibinlin
Copy link
Author

> Do you mean a struct wrapper of Response and Error?

Yes! But not like Option in rust. Request won't receive a wrapped Response. It should work in this way。

  1. Requester send a request
// request
res:= System.Root.RequestFuture(roomMgrPid, info, 1*time.Second)
  1. responser return Response
// respond
resp := &Response{
    Err: fooErr
    RetVAlue: fooRetValue
}
context.Respond(resp)
  1. requester call result()
// fooRetValue is resp.fooRetValue fooErr may be protoactor-go`s error type or resp.fooErr
fooRetValue, fooErr := res.Result()

@jidibinlin
Copy link
Author

> Response struct isn't necessary with that case.

Yes! we can use type switch to distinguish error and return value. But personally I like protoactor-go help me do this so we can.

// fooRetValue is resp.fooRetValue fooErr may be protoactor-go`s error type or resp.fooErr
fooRetValue, fooErr := res.Result()
if fooErr != nil {
// Log this error
}

// do something normal

@rogeralsing
Copy link
Collaborator

I'm all for investigating this further.
There are also an opportunity to make this Generic if needed

@Kunduin
Copy link
Contributor

Kunduin commented Jan 30, 2023

I also encountered this problem when I ported pubsub from csharp. Check out how I implemented a future-like structure at that time.

// ProduceProcessInfo is the context for a Produce call
type ProduceProcessInfo struct {
Finished chan struct{}
Err error
cancelFunc context.CancelFunc
cancelled chan struct{}
}
// IsCancelled returns true if the context has been cancelled
func (p *ProduceProcessInfo) IsCancelled() bool {
select {
case <-p.cancelled:
return true
default:
return false
}
}
// IsFinished returns true if the context has been finished
func (p *ProduceProcessInfo) IsFinished() bool {
select {
case <-p.Finished:
return true
default:
return false
}
}

The logic to wait for the completion of future and determine if there are any errors is like this ⬇️

for _, info := range infos {
<-info.Finished
suite.Assert().Nil(info.Err)
}

May be helpful for the design of future.

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

4 participants