Question about changes to ESI processing, possible feature request

[Dridi Boukelmoune]

On Thu, Jun 22, 2023 at 12:28 AM Daniel Karp <danielkarp at gmail.com> wrote:
>
> Hi, this is, I think, my first post to a varnish mailing list--I hope
> this is the right place for this.

Welcome!

> Varnish 7.3 changes the way it handles errors for Edge-side includes.
> Previously, the body of an ESI response would be included in the
> parent regardless of the status code of the ESI response. 7.3 changed
> this behavior to more closely match the ESI specification
> (https://www.w3.org/TR/esi-lang/). Now, if there is an error, the
> parent request will fail as well. The "onerror" attribute for ESIs is
> also supported (with the appropriate feature flag), allowing the
> request to continue when it would otherwise fail because of a failed
> ESI.
>
> But it isn't clear to me from the docs what happens when an ESI fails
> and include is set to "continue".
> The changelog says "any and all ESI:include objects will be delivered,
> no matter what their status might be."
> The user guide says "However, it is possible to allow individual
> <ESI:include… to continue in case of failures..."
> The Pull request for this feature says: "This changes the default
> behavior of includes, matching the ESI language specification."
> The ESI specification itself says: "If an ESI Processor can fetch
> neither the src nor the alt, it returns a HTTP status code greater
> than 400 with an error message, unless the onerror attribute is
> present. If it is, and onerror="continue" is specified, ESI Processors
> will delete the include element silently."
>
> What is the new behavior? Will the body of the response be included
> with onerror set to "continue" (which reproduces the previous
> behavior), or will the element be silently removed?

It is a good question and the short answer is that nothing is removed.

If the processing of an ESI fragment fails in VCL, and there is no
onerror="continue" kicking in, ESI delivery just stops where the parent
response was, in the middle of its body delivery.

Past VCL execution, we have the aforementioned body delivery, and
if your include fails for any reason, the _partial_ include body was part
of the parent response delivery, but again, it is interrupted.

In each case, if onerror="continue" was in effect you would likely get
a 503 guru meditation in the middle of your overall response for the
former and a missing gap for the latter.

> If it is the former, then that shouldn't be a problem for our use
> case, although it is not great for conforming to the ESI specs. But if
> the element is silently removed--or if that change is discussed to
> better conform to the standards, I have a feature request. :)

In other words, not removed, either replaced by a synthetic response
or truncated.

> We use ESI extensively for our JSON API, returning arrays of results.
> We might have something like:
> {foo: [
>     <esi 1 />,
>     <esi 2 />,
>     etc...
> ] }
> Some of those requests might be 404 responses, and our 404's have
> "null" in the body (I know, a bit hacky, but it works) so that the
> JSON remains valid. If the ESI were silently removed, that would break
> the JSON in our responses.

I have seen and done much worse, this is actually an interesting
representation of the resource in the "not found" state.

Partial delivery could mean that you get "nu" instead of "null" and
roll with it because of onerror="continue".

> But there is a better solution in the ESI Specs: The "alt" attribute.
> If varnish were to silently remove the include, and also support the
> alt attribute, this would be the cleanest solution. In that case, we
> could set the alt attribute for our ESIs to an endpoint that returns a
> 200 response and "null", where appropriate.

I don't think we support the alt attribute, or I remember pretty badly
since I introduced the initial onerror support. It has been mentioned
very recently so maybe there could be a move in this direction. An
include with src and alt attributes would likely not be streamed,
ruling out the "partial fragment" scenario.

> I hope this is clear!

I seriously doubt my answer was :)

Cheers,
Dridi