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 _______________________________________________ Pan-users mailing list Pan-users@nongnu.org https://lists.nongnu.org/mailman/listinfo/pan-users
