[leafnode-list] Re: leafnode-2.0.0.alpha20061010a snapshot available

[Martin]

On Tue, Oct 10, 2006 at 01:58:00AM +0200, Matthias Andree wrote:
> Leafnode 2.0.0.alpha20061010a is available from
> http://home.pages.de/~mandree/leafnode/beta/
> 
> NEWS:
> KNOWN BUGS:
> * The code sometimes uses timeout_client where it should use a server timeout.
> |
> 20061010a: Changes since 20060831a:
> + Documentation: in leafnode(8)'s section on groupexpire, list the
>   precedence information. Found by Clemens Fischer.

Does not work due to error handling ... =;o)

Sorry for late reply, much more sorry for not testing 20060831a! The bug
seems to have already appeared in there, if I tracked it down correctly.

|# fetchnews -vvvve -S news.cnntp.org
|
|[...]
|news.cnntp.org: de.comm.provider.status: no new articles
|news.cnntp.org: de.comm.provider.usenet: no new articles
|news.cnntp.org: de.comp.os.unix.linux.infos: no new articles
|de.newusers.infos: considering 68 articles 1691 - 1758, using XOVER
|de.newusers.infos: XOVER: 20 seen, 19 I have, 0 filtered, 1 to get
|de.newusers.infos: will fetch 1 articles
|sent ARTICLE 1692 command, in pipe: 1
|Wrong reply to ARTICLE command: "423 No such article number in this group"
|received article, in pipe: 0
|de.newusers.infos: error fetching, proceeding to next server
|[...]

"No such article number" was known. You (Matthias) released a patch and
corrected it in a following version.

The august version did not contain the checking for 2 arguments of the
server reply any more, you replaced it with checking for an error number
between 500 and 600 (which I'd consider correct):

(simple diffing of July and August, 31th)
|413c413,415
|<       if (argcount < 2 || reply / 100 == 5)
|---
|>       /* don't complain if artno missing,
|>        * 423 replies for instance don't include it, but are non-fatal.
|>        */
|>       if (reply >= 500 && reply < 600)
|414a417
|>       /* recoverable error */

You introduced a new veriable, "u":

|1139c1142
|<     unsigned long last = 0;
|---
|>     unsigned long last = 0, u;

You replaced your remark "handle error" by some real error handling:

|1259,1260c1262,1270
|<     if (getarticles(stufftoget, outstanding, f) == 0) {
|<       /* handle error */
|---
|>
|>     u = getarticles(stufftoget, outstanding, f);
|>     freefilter(f);
|>     freelist(stufftoget);
|>     if (u == 0) {
|>       ln_log(LNLOG_SERR, LNLOG_CGROUP,
|>               "%s: error fetching, proceeding to next server",
|>               g->name);
|>       return -2;
|1268,1269d1277
|<     freefilter(f);
|<     freelist(stufftoget);

The error "Wrong reply..." above seems to come from an late error-code
checking: In line 411 already this error is sent to the logfile, the
checking for the error code is done in line 415.

*If* I understand the below part right, the "proceeding to the next
server" is raised by the problem of getarticles() returning a number of
0 fetched articles.

So I "wrote" this little hack to get things working again. (This seems
like an error that can vanish any time to me, so I hurried up :) 

*****                                                  *****
*****   Please review if I break *any* other things.   *****
*****                                                  *****

The patch worked for me as far as I could see (except for "Wrong reply
..." still appears). The first run of fetchnews *with* this patch made
fetchnews fetch the whole couple of newsgroups, the second run showed
me, that even de.newusers.infos was fetched.

Maybe any logfile now shows "-1" fetched articles. I do not consider
this as severe error, so I did not use my time to dig deeper into it.

I did not indent the line correctly, since I wanted to mark it as
"unofficial".

Have fun!

Martin


The patch follows:

diff -rN -u leafnode-2.0.0.alpha20061010a/fetchnews.c leafnode-2.0.0.alpha20061011/fetchnews.c
--- leafnode-2.0.0.alpha20061010a/fetchnews.c	2006-08-15 13:31:26.000000000 +0200
+++ leafnode-2.0.0.alpha20061011/fetchnews.c	2006-10-13 20:48:21.000000000 +0200
@@ -1122,6 +1122,7 @@
 	    }
 	}
     }
+if (artno_server == 0) artno_server = -1;
     return artno_server;
 }