Opened 23 months ago

Last modified 23 months ago

#5971 enhancement new

reactor.callLater() tries to use string as number

Reported by: preaction Owned by: preaction
Priority: low Milestone:
Component: core Keywords:
Cc: Branch:
Author: Launchpad Bug:

Description

When passing a string as the first argument to callLater(), python dies with "AssertionError: 10000 is not greater than or equal to 0 seconds", which is really confusing. It would be nice if it would die with a TypeError first.

Change History (3)

comment:1 Changed 23 months ago by exarkun

  • Owner set to preaction

I'm not sure about this. callLater did not try to use a string as a number. The application that passed a string to callLater tried to use a string as a number.

There is a nearly infinite number of ways in which passing the wrong type to some API can provoke confusing or incorrect behavior. That's Python.

What alternative is there? To put type checks at the beginning of every method for every parameter, and after every method call on the returned object? That's not feasible.

comment:2 Changed 23 months ago by preaction

I was asked on the IRC channel to post this as a ticket. For the most part I agree with you, except that, in this case, Python's assert error is extremely confusing. I've tried to see if there's a way to change the message, but couldn't find one.

I did find an article on the Python wiki that suggests using exceptions in this case, because assertions can be disabled, but I imagine disabling the assertions is a feature here (though, if the type checking was an assertion, it could be disabled).

In my opinion, this is one of those issues that gets addressed on a case-by-case basis. If the code could cause irreparable damage, or if the error message that results wastes a lot of developer time, I add the check. It is a judgement call, not something that needs to be applied everywhere with a broad stroke.

Though even a mention in the docs could help the next person who has this problem.

But as you say, this is Python, strings don't get automatically converted into numbers, lesson learned, and so I'll also make sure to exercise my code better.

comment:3 Changed 23 months ago by exarkun

I was asked on the IRC channel to post this as a ticket.

Not on the dev channel. ;)

I did find an article on the Python wiki that suggests using exceptions in this case, because assertions can be disabled, but I imagine disabling the assertions is a feature here (though, if the type checking was an assertion, it could be disabled).

The use of assert here (and, really, anywhere) violates the Twisted coding standard. Precisely because they might unilaterally be removed, they should not be used at all. If you felt like replacing this code with some real checks (using if and raise) then that would probably be fine. Unit tests (see twisted.internet.test.test_time) would be required for such a change, of course.

Note: See TracTickets for help on using tickets.