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.

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.

None yet

1 participant