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

QueueProps.timeout doesn't work on both sim and tf-aws #2401

Open
tsuf239 opened this issue May 4, 2023 · 7 comments
Open

QueueProps.timeout doesn't work on both sim and tf-aws #2401

tsuf239 opened this issue May 4, 2023 · 7 comments
Labels
☁️ aws Related to Amazon Web Services support 🐛 bug Something isn't working 🙋‍♀️ help wanted Extra attention is needed 🎨 sdk SDK 🕹️ simulator Related to the `sim` compilation target

Comments

@tsuf239
Copy link
Collaborator

tsuf239 commented May 4, 2023

I tried this:

run this code (with both wing test and wing test -t tf-aws):

bring cloud;

let q = new cloud.Queue(cloud.QueueProps{timeout: 1s});

class JSHelper {
  init() {}
  extern "../external/sleep.js" inflight sleep(milli: num);
}

let js = new JSHelper();

q.add_consumer(inflight () => {
  js.sleep(2000);
});


new cloud.Function(inflight ()=> {
  // each push should result in a timeout
  q.push("foo");
  // wait for 3 seconds
  js.sleep(2000);
  // The queue should have 1 message still due to timeout
  assert(q.approx_size() == 1);
}) as "test";

This happened:

the queue had 0 messages, the message didn't go back to the queue after the timeout

in aws:
image

I expected this:

to run without problems

Is there a workaround?

No response

Component

SDK

Wing Version

No response

Wing Console Version

No response

Node.js Version

No response

Platform(s)

No response

Anything else?

No response

Community Notes

  • Please vote by adding a 👍 reaction to the issue to help us prioritize.
  • If you are interested to work on this issue, please leave a comment.
@tsuf239 tsuf239 added the 🐛 bug Something isn't working label May 4, 2023
@staycoolcall911 staycoolcall911 added good first issue Good for newcomers 🎨 sdk SDK labels May 4, 2023
@Chriscbr Chriscbr added 🕹️ simulator Related to the `sim` compilation target ☁️ aws Related to Amazon Web Services support labels May 4, 2023
@fynnfluegge
Copy link
Collaborator

fynnfluegge commented May 13, 2023

Hey @tsuf239,

I am the offender who touched the timeout and retention of queue lastly I think 😁

The default timeout of the function in simulator is 1 min, and if you specify 1s in the queue props it makes sense to me that an error is thrown, doesn't it? If you specify the timeout in your function less than queue's timeout it should work. But the question is, what should happen if the timeout of a consumer function is greater than the timeout of the queue itself? If so, to me there is something wrong in the application and throwing an error might be valid, what do you think?

But we should make sure that the defaults match this criteria.

@tsuf239
Copy link
Collaborator Author

tsuf239 commented May 14, 2023

There are two issues here actually:
1- We're letting the users discover their error only when running terraform -
we could potentially warn them before (but then we need to track it if aws are changing this limit, it's just a design decision that needs to be made).
Another thing is that the simulator doesn't have this limit (which makes sense- since we're only simulating)- and it can lead to code working on the simulator but not on the cloud (another decision to be made...)

2-The second part and the real bug here, is the timeout on the simulator- after passing the timeout, the message should go back to the Queue- see here: #165

@fynnfluegge
Copy link
Collaborator

fynnfluegge commented May 14, 2023

1: I agree 🙏

2: Have you tried to set the timeout in the QueueProps to 61s in your example? I have the feeling that the message will get pushed back to the queue in that case

Edit: I Looked up the code and got the issue again, the message is pushed back to the queue only if the consumer throws an error AND after the timeout which is triggered once after the error currently. If the consumer will timeout the message is lost, and if the timeout is very close it is ignored (as in your example). I can remember, that this was a raw workaround initially, since before the console crashed with a stack overflow when a consumer threw an error. You are right, the queue should simulate the timeout correctly, the timeout handling is not accurate now.

@fynnfluegge fynnfluegge added 🙋‍♀️ help wanted Extra attention is needed and removed good first issue Good for newcomers labels May 18, 2023
@github-actions
Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

Copy link

Hi,

This issue hasn't seen activity in 60 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

Copy link

github-actions bot commented Apr 9, 2024

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

Copy link

Hi,

This issue hasn't seen activity in 90 days. Therefore, we are marking this issue as stale for now. It will be closed after 7 days.
Feel free to re-open this issue when there's an update or relevant information to be added.
Thanks!

@github-actions github-actions bot added the Stale label Jul 11, 2024
@tsuf239 tsuf239 removed the Stale label Jul 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
☁️ aws Related to Amazon Web Services support 🐛 bug Something isn't working 🙋‍♀️ help wanted Extra attention is needed 🎨 sdk SDK 🕹️ simulator Related to the `sim` compilation target
Projects
Status: 🤝 Backlog - handoff to owners
Development

No branches or pull requests

4 participants