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

Conversation

dridi
Copy link
Member

@dridi dridi commented Apr 29, 2024

Following several VDD discussions, this patch series shows how getting a backend error could look like in VCL, and how it could be articulated in the core code.

This should check the boxes from the last consensus:

  • a plain text error message
  • a new VCL variable to get the error (beresp.error)
  • VCL constants for safe comparisons (error.*)

The first two commits are unrelated, I found a bug in libvcc while I was working on this. I will submit a pull request once I'm done with that bug (only partly fixed here) so the relevant commits to review are the ones starting with "POC".

If we still have consensus on this approach, I will find someone™ to submit something comprehensive.

All call sites pass the STRINGS type, so we can inline it directly.
This otherwise fails on a technicality that suggests the original intent
was to allow such a comparison:

    Comparison of different types: STRING '==' STRING

The STRANDS <cmp> STRING comparison is denied with a different error
message.
For now it is a mere string, but having a dedicated ERROR type leaves
the door open to properties (eg. error.foo.errno) in the future.

Error symbols are constants, not variables, but reuse the vcl-var.7
codegen infrastructure.
The same should probably be done for resp in vcl_synth. The error field
can be null, meaning there was either no error, or none was reported.
There could be a catch-all "Unknown error" in the FSM and a respective
error.unknown constant in VCL.

Considerations like return(retry) not addressed.
@dridi
Copy link
Member Author

dridi commented May 6, 2024

Bugwash summary:

  • Turn errors into a struct vrt_error { const char *type, *msg };
    • @nigoroll wanted to categorize errors (example: "backend timeout")
    • I suggested an (ERROR).type VCL attribute
    • It coud also be a list of tags (not brought up during bugwash)
  • Turn beresp.error into a proper ERROR
  • Teach $Error to vmods (add vmod_debug coverage)
  • Error messages from the documentation should be logged (I meant to initially and forgot)

Then we reevaluate the error reporting.

@dridi
Copy link
Member Author

dridi commented Jul 9, 2024

I wanted to revisit this ticket this morning and noticed that I forgot about the libvcc bug. I pushed 0c8448e...b77b709 to fix it.

@nigoroll
Copy link
Member

bugwash label removed because this ticket needs an update

@dridi
Copy link
Member Author

dridi commented Oct 14, 2024

New proposal:

struct vrt_error {
        const char *msg;
        const char *tags; /* comma-separated list of VCL identifiers */
};

Can be produced by a VMOD and put in context, to be consumed from vcl_backend_error (and probably vcl_synth too). The error can be null, like a header.

VCL defines new variables:

bereq.error

	Type: ERROR

	Readable from: vcl_backend_error

	TODO: description (has a tostring yielding `msg` or `NULL`).

bereq.error.*

	Type: BOOL

	Readable from: vcl_backend_error

	TODO: description, tells whether the error has an arbitrary tag.

We gradually start defining ERROR constants in VCL, as we increase our bereq.error coverage for at least faults happening in core code:

error.connection_refused

	Type: ERROR

	Error message: Connection refused by the backend

	Tags: backend, connect

	TODO: description (maybe mention ECONNREFUSED etc)

The error.* symbols are implicitly read-only and unrestricted in scope, in other words global constants.

In VCL you could do something like this:

sub vcl_backend_error {
        if (!bereq.error.backend || bereq.error == error.connection_refused) {
                return (retry);
        }
}

Likewise, VMODs learn to define errors in their VCC file:

$Error name, "msg", tag, tags... # XXX: implicit tag with the name of the VMOD?

And you can use them also:

import foo;

sub vcl_backend_error {
        if (bereq.error.foo && bereq.error != foo.bar) {
                return (abandon);
        }
}

edit: changed all beresp references to bereq to bind the error to the task (and only [be]req is available for the whole task, so it could be extended beyond backend_error/synth).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants