[master] 223847ea0 v1f: Log a useful HTC status

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Tue Feb 13 14:58:07 UTC 2024


commit 223847ea09a0a5440510fe8c383bdac56890dff5
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Feb 13 15:25:05 2024 +0100

    v1f: Log a useful HTC status
    
    Instead of adding hoops to the documentation, in particular to keep it
    in sync, improve the only location where we emit HTC status logs.
    
    We could also consider replacing the HTC enum with a struct, similar to
    what we did in other places. The struct symbols would be named after the
    UPPER name from the table, have a name and description fields, possibly
    replace the error number with a simple is_err field.
    
    Capturing the long description like this is less intrusive.
    
    Refs #4042

diff --git a/bin/varnishd/cache/cache_session.c b/bin/varnishd/cache/cache_session.c
index dfa2a0bf7..43d5ab60e 100644
--- a/bin/varnishd/cache/cache_session.c
+++ b/bin/varnishd/cache/cache_session.c
@@ -237,17 +237,20 @@ SES_Get_String_Attr(const struct sess *sp, enum sess_attr a)
 
 /*--------------------------------------------------------------------*/
 
-const char *
-HTC_Status(enum htc_status_e e)
+void
+HTC_Status(enum htc_status_e e, const char **name, const char **desc)
 {
+
 	switch (e) {
 #define HTC_STATUS(e, n, s, l)				\
-		case HTC_S_ ## e:	return (s);
+	case HTC_S_ ## e:				\
+		*name = s;				\
+		*desc = l;				\
+		return;
 #include "tbl/htc.h"
 	default:
 		WRONG("HTC_Status");
 	}
-	NEEDLESS(return (NULL));
 }
 
 /*--------------------------------------------------------------------*/
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 062041f49..540781821 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -436,7 +436,7 @@ void SES_Wait(struct sess *, const struct transport *);
 void SES_Ref(struct sess *sp);
 void SES_Rel(struct sess *sp);
 
-const char * HTC_Status(enum htc_status_e);
+void HTC_Status(enum htc_status_e, const char **, const char **);
 void HTC_RxInit(struct http_conn *htc, struct ws *ws);
 void HTC_RxPipeline(struct http_conn *htc, char *);
 enum htc_status_e HTC_RxStuff(struct http_conn *, htc_complete_f *,
diff --git a/bin/varnishd/http1/cache_http1_fetch.c b/bin/varnishd/http1/cache_http1_fetch.c
index 05fe46418..1a2ced1e8 100644
--- a/bin/varnishd/http1/cache_http1_fetch.c
+++ b/bin/varnishd/http1/cache_http1_fetch.c
@@ -175,6 +175,7 @@ V1F_FetchRespHdr(struct busyobj *bo)
 	double t;
 	struct http_conn *htc;
 	enum htc_status_e hs;
+	const char *name, *desc;
 
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 	CHECK_OBJ_NOTNULL(bo->htc, HTTP_CONN_MAGIC);
@@ -219,8 +220,9 @@ V1F_FetchRespHdr(struct busyobj *bo)
 			htc->doclose = SC_RX_TIMEOUT;
 			break;
 		default:
-			VSLb(bo->vsl, SLT_FetchError, "HTC %s (%d)",
-			     HTC_Status(hs), hs);
+			HTC_Status(hs, &name, &desc);
+			VSLb(bo->vsl, SLT_FetchError, "HTC %s (%s)",
+			     name, desc);
 			htc->doclose = SC_RX_BAD;
 			break;
 		}
diff --git a/bin/varnishtest/tests/c00036.vtc b/bin/varnishtest/tests/c00036.vtc
index 243339a6e..a229b74f0 100644
--- a/bin/varnishtest/tests/c00036.vtc
+++ b/bin/varnishtest/tests/c00036.vtc
@@ -27,7 +27,7 @@ logexpect l1 -v v1 -q "vxid == 1004" {
 
 	# purpose of this vtc: test the internal retry when the
 	# backend goes away on a keepalive TCP connection:
-	expect 0 1004 FetchError      {^HTC eof .-1.}
+	expect 0 1004 FetchError      {^HTC eof .Unexpected end of input.}
 	expect 0 1004 BackendClose    {^\d+ s1}
 	expect 0 1004 Timestamp       {^Connected:}
 	expect 0 1004 BackendOpen     {^\d+ s1}
diff --git a/include/tbl/vsl_tags.h b/include/tbl/vsl_tags.h
index 166d9cf6c..378916cab 100644
--- a/include/tbl/vsl_tags.h
+++ b/include/tbl/vsl_tags.h
@@ -92,7 +92,7 @@ SLTM(SessOpen, 0, "Client connection opened",
 	"\n"
 )
 /*
- * XXX generate the list of SC_* reasons (see also HTC info):
+ * XXX generate the list of SC_* reasons:
  *
  * #include <stdio.h>
  * int main(void) {
@@ -189,33 +189,9 @@ SLTM(Length, 0, "Size of object body",
 	"Logs the size of a fetch object body.\n\n"
 )
 
-/*
- * XXX generate HTC info below from tbl include
- *
- * #include <stdio.h>
- * int main(void) {
- * #define HTC_STATUS(e, n, s, l) \
- *	printf("\t\"\\t* ``%s`` (%d): %s\\n\"\n", s, n, l);
- * #include "include/tbl/htc.h"
- *	return (0);
- * }
- */
-
 SLTM(FetchError, 0, "Error while fetching object",
 	"Logs the error message of a failed fetch operation.\n\n"
-	"Error messages should be self-explanatory, yet the http connection\n"
-	"(HTC) class of errors is reported with these symbols:\n\n"
-	"\t* ``junk`` (-5): Received unexpected data\n"
-	"\t* ``close`` (-4): Connection closed\n"
-	"\t* ``timeout`` (-3): Timed out\n"
-	"\t* ``overflow`` (-2): Buffer/workspace too small\n"
-	"\t* ``eof`` (-1): Unexpected end of input\n"
-	"\t* ``empty`` (0): Empty response\n"
-	"\t* ``more`` (1): More data required\n"
-	"\t* ``complete`` (2): Data complete (no error)\n"
-	"\t* ``idle`` (3): Connection was closed while idle\n"
-	"\nNotice that some HTC errors are never emitted."
-	)
+)
 
 #define SLTH(tag, ind, req, resp, sdesc, ldesc) \
 	SLTM(Req##tag, (req ? 0 : SLT_F_UNUSED), "Client request " sdesc, ldesc)


More information about the varnish-commit mailing list