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

Missing timeout in SendSync #514

Closed
webfrank opened this issue Nov 11, 2024 · 12 comments · Fixed by #515
Closed

Missing timeout in SendSync #514

webfrank opened this issue Nov 11, 2024 · 12 comments · Fixed by #515
Assignees
Labels
enhancement question Further information is requested

Comments

@webfrank
Copy link

Bug description
PID.SendSync has a fixed timeout of 20 seconds.

How to reproduce it?
Send a message to a mailbox which will not reply.

Expected behavior
This should be a configurable parameter, with a 20 secs default value.

Screenshots

Library Version:

  • Go-Akt version: 2.7.1
  • Go version: 1.23

Additional context
I would like to know how to send a ctx.Response() which is an error. Now I'm using this code:

resp, err := d.getDevices()
if err != nil {
	ctx.Err(err)
} else {
	ctx.Response(resp)
}

but the receiver will timeout if ctx.Err(err) is used, I expect to have an error instead to signal an issue.

@webfrank webfrank added the bug Something isn't working label Nov 11, 2024
@Tochemey
Copy link
Owner

@webfrank I will take a look at the issue and get back to you. Also can you point me to the hard-coded 20 seconds path?

@Tochemey
Copy link
Owner

The reply timeout is set when creating the actor system. It is a timeout that is applied to any ask-pattern message in the system. It can be set https://github.com/Tochemey/goakt/blob/v2.7.1/actors/option.go#L68. The default value is 20 seconds as you stated.

@Tochemey
Copy link
Owner

@webfrank kindly take a look at the example in the cluster code to see how the ask-pattern works: https://github.com/Tochemey/goakt-examples/blob/main/actor-cluster/static/actors/actors.go. I will be glad to provide further explanation.

@Tochemey Tochemey added question Further information is requested and removed bug Something isn't working labels Nov 11, 2024
@webfrank
Copy link
Author

The reply timeout is set when creating the actor system. It is a timeout that is applied to any ask-pattern message in the system. It can be set https://github.com/Tochemey/goakt/blob/v2.7.1/actors/option.go#L68. The default value is 20 seconds as you stated.

Hi, I think it could be useful to set it on a per message basis, having more control on it.

@webfrank
Copy link
Author

@webfrank kindly take a look at the example in the cluster code to see how the ask-pattern works: https://github.com/Tochemey/goakt-examples/blob/main/actor-cluster/static/actors/actors.go. I will be glad to provide further explanation.

Hi, I've already implemented the examples. What I was trying to achieve is to return back an error instead of waiting timeout.

If a receiving actor wants to reply with an error the only thing could do is calling ctx.Err() but this does not cancel the context and does not returns the error. This way the caller should wait for the timeout.

@Tochemey
Copy link
Owner

@webfrank fair point. I can look into that.

@Tochemey
Copy link
Owner

@webfrank are you free for a quick call?

@webfrank
Copy link
Author

@webfrank are you free for a quick call?

Hi, sorry, dinner time. I'm CET timezone. I'll send a DM with my contacts.

@Tochemey
Copy link
Owner

@webfrank I have refactored the code a bit in this PR to address the timeout issue. You may have to use the latest commit hash when I merge it because I am not planning to cut a release tag soon until the christmas period because I am planning some changes in the library.

@Tochemey
Copy link
Owner

Tochemey commented Nov 11, 2024

  • ctx.Err(err) is recommended instead of calling panic. This will trigger the supervisor to handle the faulty actor.
  • ctx.Response is used to send the response to a request. This is used when using the ask-pattern of messaging

@Tochemey
Copy link
Owner

@webfrank kindly use this commit hash 9760b03 for the meantime. I will cut a release tag a bit later.

@Tochemey
Copy link
Owner

@webfrank kindly use the latest release 2.8.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants