Opened 7 weeks ago

Closed 7 weeks ago

Last modified 7 weeks ago

#9763 defect closed duplicate (duplicate)

Unclear type for argument to IReactorTCP.connectTCP

Reported by: Xander Desai Owned by:
Priority: normal Milestone: Python-3.x
Component: core Keywords: py3 python3 bytes string encoding connectTCP host type
Cc: xdesai@… Branch:
Author:

Description

Hi All,

While converting our code base for python3 support I believe I have stumbled upon what I hope to be a simple documentation error, but felt the need to reach out for clarity.

IReactorTCP.connectTCP takes in an argument called host. According to the doc string of this function the type of host is to be bytes. Here's the autogenerated documentation: https://twistedmatrix.com/documents/19.10.0/api/twisted.internet.interfaces.IReactorTCP.connectTCP.html

Despite this function declaring that it wants bytes for that parameter I see in several other pieces of twisted documentation that call that function with string as the type for host. In my local testing passing in either string or bytes does seem to function correctly with the happy path. However, where we have encountered an issue is with a failed DNS resolution.

Passing in a bad hostname that doesn't resolve where the type of host is string you get a nice error like this

Connection failed. Reason: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.DNSLookupError'>: DNS lookup failed: api-test.xander.
]

Passing in a bad hostname that doesn't resolve where the type of host is bytes we instead get this traceback.

Connection failed. Reason: [Failure instance: Traceback (failure with no frames): <class 'twisted.internet.error.DNSLookupError'>: <DNSLookupError instance at 0x101c1a6a8 with str error:
 Traceback (most recent call last):
  File "/Users/xdesai/junk/python_weirdness/3env/lib/python3.7/site-packages/twisted/python/reflect.py", line 448, in safe_str
    return str(o)
  File "/Users/xdesai/junk/python_weirdness/3env/lib/python3.7/site-packages/twisted/internet/error.py", line 77, in __str__
    s = '%s: %s' % (s, ' '.join(self.args))
TypeError: sequence item 0: expected str instance, bytes found
>
]

I believe there are two simple solutions here, but I don't know which one is the right one.

  1. Update the doc string for connectTCP to recommend string instead of bytes
  2. Update the __str__ method on DNSLookupError to handle bytes and strings

Hopefully this details the issue we are experiencing. I'm mostly trying to figure out what type we truly want to be using for the connectTCP call.

Please let me know if there's any more detail I can provide.

Best, Xander

ps. Here is my short script to reproduce the above issue using python 3.7 and twisted 19.10.0

from twisted.internet import reactor
from twisted.internet.protocol import ClientFactory, Protocol

from sys import stdout

class Echo(Protocol):
    def dataReceived(self, data):
        stdout.write(data)

class EchoClientFactory(ClientFactory):
    def startedConnecting(self, connector):
        print('Started to connect.')

    def buildProtocol(self, addr):
        print('Connected.')
        return Echo()

    def clientConnectionLost(self, connector, reason):
        print('Lost connection.  Reason:', reason)

    def clientConnectionFailed(self, connector, reason):
        print('Connection failed. Reason:', reason)

def main():
    host = b'api-test.xander'
    port = 80
    reactor.connectTCP(host, port, EchoClientFactory())
    reactor.run()


if __name__ == '__main__':
    main()

Change History (3)

comment:1 Changed 7 weeks ago by Jean-Paul Calderone

Author: Xander Desai

comment:2 Changed 7 weeks ago by Julian Berman

Resolution: duplicate
Status: newclosed

Hi! Thanks.

This looks likely to be a duplicate of #7956.

Feel free to lemme know if that's not the case, otherwise a fix would most definitely be welcome I'm sure.

comment:3 Changed 7 weeks ago by Xander Desai

Ah yep it looks to be the same issue. Sorry for the duplicate. I should have searched harder. Am I reading correctly that the ticket was opened 5 years ago. Would it help to create a pull request to change that doc string?

*Edit. Nvm I reread your post. I'll see if I can whip up a doc string change sometime soon :)

Last edited 7 weeks ago by Xander Desai (previous) (diff)
Note: See TracTickets for help on using tickets.