Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC: New beresp.error VCL variable #4097

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion bin/varnishd/builtin.vcl
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,13 @@ sub vcl_builtin_backend_error {
</head>
<body>
<h1>Error "} + beresp.status + " " + beresp.reason + {"</h1>
<p>"} + beresp.reason + {"</p>
<p>"};
if (beresp.error) {
set beresp.body += beresp.error;
} else {
set beresp.body += beresp.reason;
}
set beresp.body += {"</p>
<h3>Guru Meditation:</h3>
<p>XID: "} + bereq.xid + {"</p>
<hr>
Expand Down
1 change: 1 addition & 0 deletions bin/varnishd/cache/cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,7 @@ struct busyobj {

uint16_t err_code;
const char *err_reason;
const char *err_resp;

const char *client_identity;
};
Expand Down
8 changes: 5 additions & 3 deletions bin/varnishd/cache/cache_backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
#include "cache_vcl.h"
#include "http1/cache_http1.h"
#include "proxy/cache_proxy.h"
#include "vrt_obj.h"

#include "VSC_vbe.h"

Expand All @@ -60,7 +61,7 @@ static struct lock backends_mtx;

/*--------------------------------------------------------------------*/

void
const char *
VBE_Connect_Error(struct VSC_vbe *vsc, int err)
{

Expand All @@ -81,7 +82,7 @@ VBE_Connect_Error(struct VSC_vbe *vsc, int err)
break;
case ECONNREFUSED:
vsc->fail_econnrefused++;
break;
return (VRT_c_error_connection_refused());
case ENETUNREACH:
vsc->fail_enetunreach++;
break;
Expand All @@ -91,6 +92,7 @@ VBE_Connect_Error(struct VSC_vbe *vsc, int err)
default:
vsc->fail_other++;
}
return (NULL);
}

/*--------------------------------------------------------------------*/
Expand Down Expand Up @@ -158,7 +160,7 @@ vbe_dir_getfd(VRT_CTX, struct worker *wrk, VCL_BACKEND dir, struct backend *bp,
pfd = VCP_Get(bp->conn_pool, tmod, wrk, force_fresh, &err);
if (pfd == NULL) {
Lck_Lock(bp->director->mtx);
VBE_Connect_Error(bp->vsc, err);
bo->err_resp = VBE_Connect_Error(bp->vsc, err);
Lck_Unlock(bp->director->mtx);
VSLb(bo->vsl, SLT_FetchError,
"backend %s: fail errno %d (%s)",
Expand Down
2 changes: 1 addition & 1 deletion bin/varnishd/cache/cache_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,5 +82,5 @@ void VBP_Insert(struct backend *b, struct vrt_backend_probe const *p,
void VBP_Remove(struct backend *b);
void VBP_Control(const struct backend *b, int stop);
void VBP_Status(struct vsb *, const struct backend *, int details, int json);
void VBE_Connect_Error(struct VSC_vbe *, int err);
const char * VBE_Connect_Error(struct VSC_vbe *, int err);

11 changes: 11 additions & 0 deletions bin/varnishd/cache/cache_vrt_var.c
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,17 @@ VRT_r_beresp_backend(VRT_CTX)

/*--------------------------------------------------------------------*/

VCL_STRING
VRT_r_beresp_error(VRT_CTX)
{

CHECK_OBJ_NOTNULL(ctx, VRT_CTX_MAGIC);
CHECK_OBJ_NOTNULL(ctx->bo, BUSYOBJ_MAGIC);
return (ctx->bo->err_resp);
}

/*--------------------------------------------------------------------*/

VCL_VOID
VRT_u_bereq_body(VRT_CTX)
{
Expand Down
10 changes: 10 additions & 0 deletions bin/varnishtest/tests/v00019.vtc
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,16 @@ varnish v1 -errvcl {Comparison of different types: STRING '==' INT} {
}
}

varnish v1 -vcl {
import debug;
backend be none;
sub vcl_recv {
if ("string" == debug.return_strands("string")) {
return (fail("should compile"));
}
}
}

varnish v1 -errvcl {Symbol not found: 'req.http.req.http.foo'} {
backend b { .host = "${localhost}"; }
sub vcl_recv {
Expand Down
20 changes: 20 additions & 0 deletions bin/varnishtest/tests/v00072.vtc
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
varnishtest "VCL error messages"

# Instead of adding coverage for all errors here, we should probably add
# error.* coverage to existing test cases.

server s1 "" -start -break

varnish v1 -vcl+backend {
sub vcl_backend_error {
if (beresp.error != error.connection_refused) {
return (abandon);
}
}
} -start

client c1 {
txreq
rxresp
expect resp.body ~ "<p>Connection refused by the backend</p>"
} -run
22 changes: 22 additions & 0 deletions doc/sphinx/reference/vcl_var.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,15 @@ beresp.filters
``beresp.do_*`` switches is a VCL error.


beresp.error

Type: STRING

Readable from: vcl_backend_error

TODO: description (change type to ERROR?)


.. _beresp.grace:

beresp.grace
Expand Down Expand Up @@ -1899,3 +1908,16 @@ storage.<name>.used_space
Used space in the named stevedore. Only available for the malloc
stevedore.

Error messages
--------------

TODO: general introduction.

error.connection_refused

Type: ERROR

Error message: Connection refused by the backend

TODO: description (maybe mention ECONNREFUSED etc)

1 change: 1 addition & 0 deletions include/vrt.h
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ typedef unsigned VCL_BOOL;
typedef int64_t VCL_BYTES;
typedef vtim_dur VCL_DURATION;
typedef const char * VCL_ENUM;
typedef const char * VCL_ERROR;
typedef const struct gethdr_s * VCL_HEADER;
typedef struct http * VCL_HTTP;
typedef void VCL_INSTANCE;
Expand Down
12 changes: 11 additions & 1 deletion lib/libvcc/generate.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,12 @@ def emit_var(self):
fo.write('\tsym->rname = "HDR_')
fo.write(self.nam.split(".")[0].upper())
fo.write('";\n')
elif self.rd and self.typ != "HEADER":
elif self.typ == "ERROR":
emsg = " ".join(self.wr)
self.wr = []
fo.write('\tsym->rname = "VRT_c_%s()";\n' % cnam)
fh.write("#define VRT_c_%s() \"%s\"\n" % (cnam, emsg))
elif self.rd and self.typ != "HEADER" and self.typ != "ERROR":
fo.write('\tsym->rname = "VRT_r_%s(ctx)";\n' % cnam)
varproto("VCL_" + self.typ + " VRT_r_%s(VRT_CTX)" % cnam)
fo.write("\tsym->r_methods =\n")
Expand Down Expand Up @@ -266,6 +271,11 @@ def parse_var(ln):
for i in j[2:]:
vu.append(i.strip(",."))
continue
if j[0] == "Error" and j[1] == "message:":
vt = "ERROR"
vr = ["all"]
vw = j[2:] # stash error message in write permissions
continue
if j[0] == "Alias" and j[1] == "of:":
va = j[2]
continue
Expand Down
21 changes: 12 additions & 9 deletions lib/libvcc/vcc_expr.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,19 +288,18 @@ vcc_expr_tobool(struct vcc *tl, struct expr **e)
*/

static void
vcc_expr_tostring(struct vcc *tl, struct expr **e, vcc_type_t fmt)
vcc_expr_tostring(struct vcc *tl, struct expr **e)
{
const char *p;
uint8_t constant = EXPR_VAR;

CHECK_OBJ_NOTNULL(*e, EXPR_MAGIC);
assert(fmt == STRINGS || fmt->stringform);
assert(fmt != (*e)->fmt);
assert((*e)->fmt != STRINGS);

p = (*e)->fmt->tostring;
if (p != NULL) {
AN(*p);
*e = vcc_expr_edit(tl, fmt, p, *e, NULL);
*e = vcc_expr_edit(tl, STRINGS, p, *e, NULL);
(*e)->constant = constant;
(*e)->nstr = 1;
} else {
Expand Down Expand Up @@ -816,7 +815,7 @@ vcc_expr5(struct vcc *tl, struct expr **e, vcc_type_t fmt)
ERRCHK(tl);
/* Unless asked for a HEADER, fold to string here */
if (*e && fmt != HEADER && (*e)->fmt == HEADER) {
vcc_expr_tostring(tl, e, STRINGS);
vcc_expr_tostring(tl, e);
ERRCHK(tl);
}
return;
Expand Down Expand Up @@ -1071,9 +1070,9 @@ vcc_expr_add(struct vcc *tl, struct expr **e, vcc_type_t fmt)
} else if (tk->tok == '+' &&
((*e)->fmt == STRINGS || fmt == STRINGS)) {
if ((*e)->fmt != STRINGS)
vcc_expr_tostring(tl, e, STRINGS);
vcc_expr_tostring(tl, e);
if (e2->fmt != STRINGS)
vcc_expr_tostring(tl, &e2, STRINGS);
vcc_expr_tostring(tl, &e2);
if (vcc_islit(*e) && vcc_isconst(e2)) {
lit = vcc_islit(e2);
*e = vcc_expr_edit(tl, STRINGS,
Expand Down Expand Up @@ -1187,6 +1186,10 @@ cmp_string(struct vcc *tl, struct expr **e, const struct cmps *cp)
tk = tl->t;
vcc_NextToken(tl);
vcc_expr_add(tl, &e2, STRINGS);
if (e2->fmt != STRINGS && e2->fmt->stringform) {
/* NB: no concatenation processed by vcc_expr_add() */
vcc_expr_tostring(tl, &e2);
}
ERRCHK(tl);
if (e2->fmt != STRINGS) {
VSB_printf(tl->sb,
Expand Down Expand Up @@ -1420,7 +1423,7 @@ vcc_expr0(struct vcc *tl, struct expr **e, vcc_type_t fmt)
return;

if ((*e)->fmt != STRINGS && fmt->stringform)
vcc_expr_tostring(tl, e, STRINGS);
vcc_expr_tostring(tl, e);

if ((*e)->fmt->stringform) {
VSB_printf(tl->sb, "Cannot convert type %s(%s) to %s(%s)\n",
Expand All @@ -1431,7 +1434,7 @@ vcc_expr0(struct vcc *tl, struct expr **e, vcc_type_t fmt)
}

if (fmt == BODY && !(*e)->fmt->bodyform)
vcc_expr_tostring(tl, e, STRINGS);
vcc_expr_tostring(tl, e);

if (fmt == BODY && (*e)->fmt->bodyform) {
if ((*e)->fmt == STRINGS)
Expand Down
7 changes: 7 additions & 0 deletions lib/libvcc/vcc_types.c
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ const struct type ENUM[1] = {{
.tostring = "",
}};

const struct type ERROR[1] = {{
.magic = TYPE_MAGIC,
.name = "ERROR",
.stringform = 1,
.tostring = "(\v1)",
}};

const struct type HEADER[1] = {{
.magic = TYPE_MAGIC,
.name = "HEADER",
Expand Down