Skip to content

Commit

Permalink
v1f: Log a useful HTC status
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dridi committed Feb 13, 2024
1 parent 7536a40 commit 223847e
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 34 deletions.
11 changes: 7 additions & 4 deletions bin/varnishd/cache/cache_session.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}

/*--------------------------------------------------------------------*/
Expand Down
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache_varnishd.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 *,
Expand Down
6 changes: 4 additions & 2 deletions bin/varnishd/http1/cache_http1_fetch.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion bin/varnishtest/tests/c00036.vtc
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
28 changes: 2 additions & 26 deletions include/tbl/vsl_tags.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 223847e

Please sign in to comment.