Added await so the limiter actually limits messages instead of ignoring the Promise

[clibequilibrium]

This PR fixes the issue where the Rate Limiter not actually limits actually because the Promise is not awaited so the messages are still going through.

[MellowYarker]

Hey @clibequilibrium, it looks like checkLimit() doesn't actually return a promise, so there's nothing to await here. Instead, it checks if the limiter is cooling down, and if so, it blocks the message from being sent.

If it's not cooling down, we set inCooldown = true (preventing a subsequent message from being sent), and then schedule an async task (callLimiter()) that will set inCooldown = false after some period of time.

Does that make sense?

---

Out of curiosity, did you observe messages not being rate limited properly? I don't think any of the rate limiting code has changed recently, though I would need to take a closer look.

[clibequilibrium]

Hey @clibequilibrium, it looks like checkLimit() doesn't actually return a promise, so there's nothing to await here. Instead, it checks if the limiter is cooling down, and if so, it blocks the message from being sent.

If it's not cooling down, we set inCooldown = true (preventing a subsequent message from being sent), and then schedule an async task (callLimiter()) that will set inCooldown = false after some period of time.

Does that make sense?

Out of curiosity, did you observe messages not being rate limited properly? I don't think any of the rate limiting code has changed recently, though I would need to take a closer look.

Hello ! Thank you for your reply.

On my end I use typescript and since the function is market as async , returning false or true automatically converts it to Promise. So yes I tested it and since it was not awaiting the Promise the condition was never false, because it is always returns a Promise which is not null.

On side note, awaiting the Promise increased my DO duration because DO waits for the cooldown.
So yes on my end using typescript the messages were not awaited unless I add await keyword.

[clibequilibrium]

So converting it to non async and not awaiting this.callLimiter worked

checkLimit(): boolean {
		if (this.inCooldown) {
			return false;
		}
		this.inCooldown = true;
		this.callLimiter();
		return true;
	}

[MellowYarker]

Ahhh, I see, it looks like checkLimit() was marked async in the branch that converts to typescript. It's worth noting that the Typescript PR was submitted by an external contributor and we haven't reviewed it, so there may be unexpected changes in behavior.

I'm going to close this PR, since it seems to be a merge conflict issue between master and the typescript branch. Thanks for opening this!

[clibequilibrium]

Ahhh, I see, it looks like checkLimit() was marked async in the branch that converts to typescript. It's worth noting that the Typescript PR was submitted by an external contributor and we haven't reviewed it, so there may be unexpected changes in behavior.

I'm going to close this PR, since it seems to be a merge conflict issue between master and the typescript branch. Thanks for opening this!

Ah you are totally right ! The fault is on me 😁 I see that the master does not have the function marked as async . My bad !

[kentonv]

I went ahead and committed an update to #28 to revert checkLimit() back to being synchronous, in the hopes of avoiding confusion for anyone else who might choose to use that branch.