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

Fallback-aware failure handling for data-bridge #37

Merged
merged 4 commits into from Dec 12, 2018

Conversation

Projects
None yet
3 participants
@scypio
Copy link
Contributor

scypio commented Dec 10, 2018

Implementation of new knotx feature (fragment processing faiilure handling) within data bridge

Description

New failure handling feature was implemented in Cognifide/knotx#466
This PR allows data bridge to take advantage of the new approach and handle data bridge failures in a more robust way.

Motivation and Context

See core feature documentation

Screenshots (if appropriate)

Upgrade notes (if appropriate)

Requires Cognifide/knotx#466
Integration tests to be added to knotx-stack project

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I hereby agree to the terms of the Knot.x Contributor License Agreement.

.onErrorReturn(e -> {
LOGGER.error("Fragment processing failed. Cause:{}\nRequest:\n{}\nFragmentContext:\n{}\n", e.getMessage(), request.getClientRequest(), fragmentContext);
fragmentContext.fragment().failure(DataBridgeKnotProxy.SUPPORTED_FRAGMENT_ID, e);
if (!fragmentContext.fragment().fallback().isPresent()) {

This comment has been minimized.

@tomaszmichalak

tomaszmichalak Dec 10, 2018

Contributor

I think it should not happen here. Data Bridge should set error on fragment. Then at the end Assembler should check fallbacks and do some logic if the fallback is not defined.

This comment has been minimized.

@tomaszmichalak

tomaszmichalak Dec 10, 2018

Contributor

Data Bridge is not responsible for template evaluation, it adds the JSON object from the data source to Knot.x Context.

This comment has been minimized.

@scypio

scypio Dec 11, 2018

Contributor

The main idea behind this PR is to have a different way of processing errors for fragments, depending on Fragment configuration.

No fallback configured -> error handling happens on Knot level.
Fallback configured -> mark fragment as failed, error handling on Assembler level.

I added a dedicated handleError methot to FragmentProcessor to make this idea more comprehensible.

@@ -49,9 +52,23 @@ public FragmentProcessor(Vertx vertx, DataBridgeKnotOptions options) {
.doOnNext(this::traceService)
.flatMap(serviceEntry ->
fetchServiceData(serviceEntry, request).toObservable()
.map(serviceEntry::getResultWithNamespaceAsKey))
.map(serviceEntry::getResultWithNamespaceAsKey)
.doOnError(e -> storeErrorInFragment(fragmentContext.fragment(), e, serviceEntry.getName()))

This comment has been minimized.

@tomaszmichalak

tomaszmichalak Dec 10, 2018

Contributor

Like it!

scypio added some commits Dec 11, 2018

@Skejven Skejven merged commit bff475b into Knotx:master Dec 12, 2018

2 checks passed

CodeFactor No issues found.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment