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

Fragment processing faliure handling - fallback strategies #466

Closed
scypio opened this issue Oct 18, 2018 · 5 comments
Closed

Fragment processing faliure handling - fallback strategies #466

scypio opened this issue Oct 18, 2018 · 5 comments

Comments

@scypio
Copy link
Contributor

scypio commented Oct 18, 2018

Problem statement
Pages may contain multiple knotx snippets. Failure in processing a single snippet may result in the whole page returning 500 Error respononse.

Let's assume that a page contains two snippets:

<knotx-snippet data-knotx-knots="databridge,handlebars" 
  data-knotx-databridge-name="important-service">
      {{#if _result}}
        <h2>{{_result.count}}</h2>
      {{/if}}
</knotx-snippet>

<knotx-snippet data-knotx-knots="databridge,handlebars" 
  data-knotx-databridge-name="unstable-service">
      {{#if _result}}
        <h2>{{_result.count}}</h2>
      {{/if}}
</knotx-snippet>

When databridge / unstable-service fails the whole rendering process is terminated with a 500 Error.

Proposed solution
I would like to:

  • have the ability to mark a processed fragment as failed
  • have the ability to define a fallback strategy that defines markup that can be rendered when fragment processing fails
  • handlebars knot should not process fragments that were marked as failed
  • assembler should apply fallback strategy for failed nodes

Blank fallback strategy would just skip rendering of the failed snippet altogether.
Static fallback strategy would render a static markup defined as a snippet attribute.

For example, should following fragment fail:

<knotx-snippet data-knotx-knots="databridge,handlebars" 
  data-knotx-databridge-name="unstable-service"
  data-knotx-fallback="No data available">
      {{#if _result}}
        <h2>{{_result.count}}</h2>
      {{/if}}
</knotx-snippet>

the text No data available would be rendered as a fallback.

Alternatives
I wondered if fallback could be defined as an embedded node,

<knotx-snippet data-knotx-knots="databridge,handlebars" 
  data-knotx-databridge-name="unstable-service">
  <knotx:fallback>
    <p class="warning">No data available</p>
  </knotx:fallback>
      {{#if _result}}
        <h2>{{_result.count}}</h2>
      {{/if}}
</knotx-snippet>

I decided I would rather not process the internals of the snippet thou.

Additional context
Once the fallback feature is implemented in the knotx core we would like to take advantage of it in the data-bridge module.

e: edited to rename the fallback attribute to just "data-knotx-fallback"

@marcinczeczko
Copy link
Contributor

@scypio thanks for the ticket. Looks great. I'd add following statements/design decisions regarding implementation:

Each Knot that processes Fragments:

  1. should have an ability to flag the eligible fragment as failed in case any failure happened during processing of one fragment
  2. should ignore incoming fragments that were flagged as failed by other Knots processed it before
  3. should at least log to the logger clear error message saying that snippet processing failed for a given page URL, in the given Knot, and/or corresponding stack-trace if it was exception
  4. processing of single fragment should have a common place/API so that any custom developed Knot could easily follow it when implementing its own error handling

@marcinczeczko
Copy link
Contributor

marcinczeczko commented Oct 18, 2018

@scypio Additionally:

  1. Splitter should extract any fallbacks snippet contains and store it on the Fragment
  2. Assembler is to consume failure flag on the given fragment, and if it's failed follows the strategy for the given fragment: e.g. ignore appending fragment output to the client response (empty strategy), or appends the static failure (that's already in the Fragment, thanks to the splitter)

@malaskowski
Copy link
Member

@scypio, @marcinczeczko few thoughts/designs from me:

I like very much the idea of having a place in markup to define the fallback. I agree that this should be later consumed as a separate Fragment by Knots and that it can be reused among other snippet fragments as the fallback content.
The trick here is to introduce a new fragment type in a smart way, that will enable further extensions to be added to the Knot.x easily in the future. What we currently have, is FragmentSplitter implementation that distinguish Knot.x fragments and raw HTML fragments. I suggest keepieng to this simple idea and extend the mechanics that enrich Fragment data by its type using e.g. ServiceLoader.
This would look like this:

  • By default all dynamic Knot.x fragments are defined as: <knotx:XYZ> where knotx is the namespace that will be used by the splitter to extinguish dynamic fragments that will be later processed (everything other is RAW) and XYZ is the type of the dynamic fragment (e.g. snippet for existing functionality, fallback for the feature you want to implement and maybe some auth or other types in the future).
  • Each dynamic type would have it's factory added by ServiceLoader (thanks to that registering new dynamic type will not require releasing new version of Knot.x and will be easy to customize).
  • FragmentFactory interface will define a method toFragment(MatchResult result, String markup)
  • Fragment can change and be more generic to support other data structures (names to TBD), e.g.:
class Fragment {
  String content;
  String type;
  JsonObject attributes;
  ...
}

Example:

  1. Existing snippet type will no longer store list of knots in the list in Fragment. Instead it will be stored in the attributes field. SnippetFragmentFactory implementation will make sure of creating proper Fragment instance and setting
   type = "snippet";
   attributes.put("knots", new JsonArray(knots));
  1. FallbackFragmentFactory can do something like
   type = "fallback";
   attributes.put("fallbackId", fallbackId)
  • Using here JsonObject have additional advantage of easy serializing and deserializing messages on the Event Bus.

@pun-ky
Copy link

pun-ky commented Nov 7, 2018

fallback strategies looks nice but still extending snippets based on markup syntax is not a way to go in my opinion / not enough universal, see #462 (comment)

how about

{{!--#knotxSnippet:productsServerSide(knots="databridge,handlebars", databridge-name="someSearch")--}}`
(ok)
{{!--!knotxSnippet:productsServerSide--}}
(fallback)
{{!--/knotxSnippet:productsServerSide--}}

based on {{#each ...}}...{{else}}...{{/each}}

<script type="application/json">
{{!--#knotxSnippet:productsServerSide(knots="databridge,handlebars", databridge-name="someSearch")--}}`
{{{_response}}}
{{!--!knotxSnippet:productsServerSide--}}
{
    "results": [],
    "numFound": 0
}
{{!--/knotxSnippet:productsServerSide--}}
</script>

@malaskowski
Copy link
Member

Fallback is totally re-worked in Knot.x 2.
Please see https://github.com/Knotx/knotx-fragments-handler/blob/master/core/README.md for more details

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

No branches or pull requests

4 participants