Skip to content

Add a message queue to prevent flooding #17

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShadowNinja
Copy link
Collaborator

Most users of LuaIRC have to implement their own queue, so I added one to the library.

@JakobOvrum
Copy link
Owner

This functionality is intentionally not in LuaIRC. I think it belongs in a higher level of abstraction. Adding this to LuaIRC reduces the generality of the library by forcing the user to use a message queue, and it also forces the user to use this specific kind of message queue. I suppose it could be added as an optional adapter object, but even then its exact approach to queueing should be well documented (and probably customizable).

As a side note, delaying PONG is probably not a good idea under any circumstance.

@ShadowNinja
Copy link
Collaborator Author

This is not forced, meta:send() still exists and works the same, so old queues will still work properly, and if you want your own queue (say, to give messages priority) you and easily add it and simply not use this queue.
As for PONGs: Any messages put in the queue are guarenteed to be sent (assuming you aren't disconnected) and you are normally given a few minutes to reply to PINGs. So, unless you just queued a few hundred messages, the PONG should arrive with plenty of time to spare.
Of course you could add the PONG to the front of the queue to prevent even this scenario from causing a issue. However the PONG should be queued in case you are busy sending messages, to prevent getting disconnected for excess flood.

@soccermitchy
Copy link

What I think would help is if you make something like a meta:queueEnabled functon, sets and gets if the queue is enabled. For example: meta:queueEnabled(true) would enable it, meta:queueEnabled() returns if it's enabled or not.

@JakobOvrum
Copy link
Owner

What I think would help is if you make something like a meta:queueEnabled functon, sets and gets if the queue is enabled. For example: meta:queueEnabled(true) would enable it, meta:queueEnabled() returns if it's enabled or not.

There's no reason for it to be an integral part of the IRC object. It can just exist as a separate object that operates on an IRC object. Only clients that need the queue functionality would need the separate queue object.

@soccermitchy
Copy link

We could also have it as another branch.

@JakobOvrum
Copy link
Owner

Maintaining two separate branches is a maintenance nightmare. I don't see what advantages it has over just having a separate queue object.

@ShadowNinja
Copy link
Collaborator Author

So something like this would be good?

local conn = irc.new([...])
conn = irc.Queue(conn)
conn:connect([...])
conn:send("TEST")
conn:queue("TEST2")

Unfourtunately getting connect() to use the queue will be difficult this way...

@JakobOvrum
Copy link
Owner

I don't see why connect would need to use the queue, to be honest.

edit:

But yes, I think that would be really good! :)

@ShadowNinja
Copy link
Collaborator Author

Sorry for not responding for so long, but I came up with a solution.
Simply require irc.queue if you want the queue. meta:queue is a redirect to meta:send if the queue isn't enabled. This allows the core to use the queue properly too.
Some messages aren't queued (QUIT, because at that point it doesn't matter if you flood-quit and otherwise the server won't get the message; and meta:whois, because it's synchronous)

Example:

require("irc")
require("irc.queue")
local conn = irc.new([...])
conn:connect([...])
conn:send("TEST") -- Sent instantly
conn:queue("TEST2") -- Sent at the next think()

@JakobOvrum
Copy link
Owner

Silently altering the behaviour of the irc module based on whether irc.queue is imported or not seems like a violation of the principle of least surprise (e.g. imagine a complex program where the two modules are required in different source files!). Good design is usually boring design.

@ShadowNinja
Copy link
Collaborator Author

Importing irc.queue doesn't override meta:send. You also have to change conn:send to conn:queue in your code.
Therefore this shouldn't cause anything unexpected to happen.
This allows you to use meta:send if you really have to send something immediately.

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

Successfully merging this pull request may close these issues.

3 participants