t.w.http.Request.finish should log error instead of raising RuntimeError

[psykidellic]

Calling Request.finish on a closed connection raises RuntimeError but its not forwarded to corresponding d.errback().

At this point of operation, there is nothing much anybody can do so it would be just better to log the message instead of raise an exception.

[psykidellic]

Find attached a patch for same.

It applies to trunk at 31585. I have implemented the changes as mithrandi asked in bug 4901.

[psykidellic]

Patch

[exarkun]

At this point of operation, there is nothing much anybody can do

Woah, what? There is certainly a better thing you can do - don't call write.

This was introduced in #4317. Now that the feature is in a release - 11.0 - I wonder who wins by removing it from the next one. There's always the possibility that you'll get an exception if you write to a finished request

but its not forwarded to corresponding d.errback().

I don't follow this. What Deferred do you expect to errback when this situation is encountered? The exception is raised at application code, so application code can do whatever it wants with it. There's no notifyOnFinish Deferred to fire because the request already fired those - because this exception is only raised when the request is finished already.

As far as the patch goes, it seems like a fine implementation of the proposed change. I am unconvinced about the change, though.

[jonathanj]

Following the conversation from #4901, it looks as though the XML-RPC code should be using notifyFinish, instead of just calling Request.finish, to avoid the exception in the first place.

I don't think removing the exception that occurs when calling finish on a finished Deferred is something worth doing.

[exarkun]

It seems like this has been sitting here for long enough. :) Going by the previous two comments, the bug here is actually invalid. #4901 should be resolved as jonathanj suggested, by tracking the request connection state and giving up early if necessary.