How to catch 422 errors and rollback operations in one store

[cibernox]

I have an offline-first application in which I need to catch errors, check the error code and react accordingly.

This application has two stores, the remote one is a JSONAPISource. The backup is a IndexedDBSource. Changes propagate to both sources so I have eventual consistency and everything is non-blocking.
I've added and configured @orbit/indexeddb-bucket successfully. It all works well in the happy path.

The problem happens when the user performs some operation that is added to the bucket and then applied to both sources. The IDB source always succeeds but sometimes the server can reject the request. Typically that will be because of a 422 error (there are other situations but this is the main one).

When that happens, since invalid data will continue to be invalid in the future, I want to remove that operation from the bucket so it's never retried and rollback the changes on the stores it has been applied to (the IDB and the in-memory store).

I manager to hack something that works ok when I have connection:

    return new MySyncStrategy({
      name: 'store-remote-update-optimistic',
      source: 'store',
      on: 'beforeUpdate',

      target: 'remote',
      blocking: false,
      action() { /* some uninteresting stuff */ },
      catch(e, transform) {
        console.warn('[ERROR UPDATING REMOTE SOURCE]', e); // eslint-disable-line
        if (e.response.status === 422) {
          this.target.requestQueue.skip();
          this.source.requestQueue.skip();
          let inverseOperations = this.source.getInverseOperations(transform.id);
          let inverseTransform = {
            id: Schema.prototype.generateId('transform'),
            operations: inverseOperations,
            options: undefined
          };
          this.source.sync(inverseTransform);
        }
      }
    });

I'm pretty sure this is not ideal but it kind of works.

However, if when saving the invalid data the user is offline, the request fails with a 500 and the request stays in the bucket to be retried the next time the user reloads the page.

In that situation, when I refresh the page with the connection enabled, Orbit.js retries the uncommitted operation in the bucket that this time reaches the server which responds with a 422 error, but that error doesn't trigger the above code!

I assumed I should be handled the error at a lower level and tried to write request strategy:

new RequestStrategy({
  source: 'remote',
  on: 'pushFail',
  action() {
    debugger;
  },
  catch() {
    debugger;
  }
})

I couldn't get the code to hit the debuggers, so I wondered if there is something off in my config.

I'm using ember-orbit, so I working under the assumption that my strategies are automatically registered by placing them in the right folder.

[dgeb]

Essentially what you want to do is perform the equivalent of a revert commit in git, which is what you've pieced together in your catch handler. However, it should probably be applied with an update request instead of sync, together with an option that indicates that it's reverting a particular transform (by id). And then your beforeUpdate strategy could key off this option and remove the failed transform from the remote source's requestQueue and then restart the processing of the queue.

There are a lot of pieces here, most of which I'd like to standardize, since this is a common failure scenario. Basically, I'm finding that almost every git command has a useful equivalent that should find its way into orbit.

[cibernox]

@dgeb however that still leaves something unexplained.

In that situation, when I refresh the page with the connection enabled, Orbit.js retries the uncommitted operation in the bucket that this time reaches the server which responds with a 422 error, but that error doesn't trigger the above code!

What I mean is that the above catch code is called when the request fails immediately (the user is online). But if the user is offline and the request is retried when the user reloads the page, the request is made and errors with a 422 as usual but this code is not rescued there.
To rescue for from that situation I had to write this even hackier code:

import JSONAPISource, { JSONAPISerializer } from '@orbit/jsonapi';
import config from '../config/environment';
import { getOwner } from '@ember/application';

class CustomSerializer extends JSONAPISerializer {
  resourceKey(type) {
    return (type === 'session' || type === 'project') ? 'remoteId' : 'id';
  }
}
export default {
  create(injections = {}) {
    injections.name = 'remote';
    injections.namespace = config.apiEndpoint;
    injections.SerializerClass = CustomSerializer;
    let source = new JSONAPISource(injections);
    // This hack is the best I could figure out, but it's hacky and
    // not a good solution long term. See https://github.com/orbitjs/orbit/issues/599
    source.handleFetchResponseError = function(response, data) {
      if (response.status === 422 || response.status === 403 || response.status === 404) {
        let owner = getOwner(injections);
        let store = owner.lookup('service:store');
        this.requestQueue.skip();
        store.requestQueue.skip();
      }
      return JSONAPISource.prototype.handleFetchResponseError.call(this, response, data);
    }

    return source;
  }
};

It seems off that there isn't a single point where I can catch and revert the operation in both scenarios (and having to override handleFetchResponseError also seems a bit low level).

[dgeb]

@cibernox I agree that override should not be needed (and certainly not there), so let's try to reproduce this situation in an integration test. I will add a new private package to the repo for full-app style integration tests that recreate scenarios like this across multiple orbit packages.

[cibernox]

@dgeb Once you've added the package I can help reproducing our scenario with IDB and IDB-bucket.

[dgeb]

@cibernox I've added the integration-tests package in #609

I know you're a bit busy these days (👶) but let me know when you have a chance to reproduce your scenario :)

[cibernox]

@dgeb I created #613 adding one test.

Something that surprised me is that despite of the catch method receiving the error, the entire test fails because a promise failed during the test. Is that something we can opt out for tests we expect that? The error is raised in handleFetchResponseError, but if I rescue the error the catch method will never be invoked.

[dgeb]

As mentioned in #613, this seems to have been resolved. Of course, please open a fresh issue if there's anything outstanding.