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

Support socket timeout in SysLogHandler to prevent indefinite blocking on socket creation #126400

Open
hodamarr opened this issue Nov 4, 2024 · 6 comments
Assignees
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@hodamarr
Copy link

hodamarr commented Nov 4, 2024

Feature or enhancement

Proposal:

Hi,
In the current implementation of the SysLogHandler class from the logging.handlers module, if the specified address does not respond or is unreachable, the createSocket method may block indefinitely. This is problematic because there is no way to set a socket timeout during the connection's creation without overriding the entire createSocket method.

The only way to add timeout is to override the whole createSocket method (if I use super().createSocket() it will get stuck on this line)

my suggestion:

class SysLogHandler(logging.Handler):
    def __init__(self, address=('localhost', SYSLOG_UDP_PORT), 
                 facility=LOG_USER, socktype=None, timeout=None):
        # Existing initialization
        self.timeout = timeout  # New timeout parameter
        self.createSocket()
    
    def createSocket(self):
        # Existing code ....
        try:
            sock = socket.socket(af, socktype, proto)
            **if self.timeout:
                     sock.setttimeout(self.timeout)**
            # Existing code ........

Thanks

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Linked PRs

@hodamarr hodamarr added the type-feature A feature request or enhancement label Nov 4, 2024
@Zheaoli
Copy link
Contributor

Zheaoli commented Nov 4, 2024

If I remember correct, the SysLog is based on the UDP protocol. I'm sure the timeout is useful for UDP

@hodamarr hodamarr closed this as completed Nov 4, 2024
@hodamarr hodamarr reopened this Nov 4, 2024
@hodamarr
Copy link
Author

hodamarr commented Nov 4, 2024

If I remember correct, the SysLog is based on the UDP protocol. I'm sure the timeout is useful for UDP

Syslog uses both UDP and TCP. A timeout is crucial to avoid blocking the process if the service is unreachable or slow to respond. In my case, I'm using TCP, and the server is unreachable, causing my process to be blocked indefinitely. Adding timeout support to SysLogHandler would allow the application to fail gracefully in such scenarios, instead of hanging due to network issues.

@picnixz picnixz added the stdlib Python modules in the Lib dir label Nov 6, 2024
@hodamarr
Copy link
Author

Hey @picnixz I can take care of this—could you assign me to it?

@picnixz
Copy link
Contributor

picnixz commented Nov 10, 2024

Before starting the implementation, I wonder whether this should count as a bug fix or not (and whether there is another way to fix it). So let's ask @vsajip first.

@hodamarr
Copy link
Author

Sure, let me know when its okay to open a pull request.
@vsajip @picnixz

@vsajip
Copy link
Member

vsajip commented Nov 10, 2024

Yes, this seems a reasonable feature to add. You're welcome to submit a PR with docs/tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-feature A feature request or enhancement
Projects
Development

No branches or pull requests

4 participants