Re: [Pan-users] RFC versus sever versus client

[Per Hedeland]

On 2016-11-13 13:07, Rhialto wrote:
> On Sat 12 Nov 2016 at 16:58:32 +0100, Per Hedeland wrote:
>> ...and in case there is interest in it, the attached patch for Pan does
>> just that - I've tested it against what I believe is the server the OP
>> is referring to, in any case it is one that does return 500 for MODE
>> READER.
> 
> How likely is it that more commands in the future will require similar
> treatment? As it is for one command now, this very specific hack works,
> but it won't be a great solution for the general case (if needed).

Agreed. I'm not sure about the answer to the question - looking at the
currently implemented commands, I think that MODE READER is unique in
that a 500 response isn't an actual problem - for the others, I think it
really should be considered fatal. But of course additional commands may
be added in the future.

> Now personally I'm not a fan of abstracting things when it is not yet
> needed but when it is complicated, so I'd propose that if a more general
> solution turns out to be indeed complicated and hard to follow, that it
> isn't done (yet).

I can certainly imagine solutions that are "cleaner" without being
complicated or hard to follow, they will just take rather more effort
(and "risk") to implement. Basically amounting to having the caller
handle the error, either as you suggest below, stating up-front that
(some) errors can be ignored, or getting to deal with the error in the
result.

As far as I can see though, either way would require significant
restructuring of the code, in particular for the relevant handshake()
function, where (including the "empty" initial one) 3 commands and
replies are "bundled", and only one of them may fail. Though maybe
"unbundling" MODE READER and having the caller of handshake() do that
separately could be a way.

> (And now that I am looking at the code in nntp.cc, I see some complexity
> that I don't see the need for (yet?). In functions like
> list_newsgroups(), a command is queued, and then the first command from
> the queue is sent to the server. Who guarantees that the queue was empty
> before, and that this is the just-queued command? Yes, the _listener
> member is set with just this assumption. Either the assumption should be
> checked, or better yet, if possible, the whole queue should be removed
> and replaced with a single ""current command". That also makes it simple
> to add a flag for "ignore any errors on this command".)

I don't know if there is an actual problem there, given how the functions
are used - in any case many of them depend on queueing up multiple
commands, in particular prepending a GROUP command to the "actual" one.
But having a more structured queue than a list of strings would
certainly help for a "cleaner" solution. Note though that in this case,
I really think it is *only* the 500/ERROR_CMD_NOT_UNDERSTOOD error that
should be ignored. The others indicate that the server actually
"understood" the command but rejected it - this could e.g. be the case
for a "feeder-only" server that doesn't provide reader service at all,
and Pan should really treat that as a fatal error.

--Per