orbitjs / orbit

Composable data framework for ambitious web applications.
https://orbitjs.com
MIT License
2.33k stars 134 forks source link

Optimistic Error Handling Got TypeError Not NetworkError #989

Open hendra opened 1 year ago

hendra commented 1 year ago

When implement the optimistic strategies for OrbitJS offline I always got error "TypeError" instead of "NetworkError" inside the RequestStrategy updateFail action block.

image

Here is my coordinator code (see my comment inside the RequestStrategy updateFail block) :

// Back up data to IndexedDB
coordinator.addStrategy(
  new SyncStrategy({
    source: "memory",
    target: "backup",
    blocking: true,
  })
);

// Sync all changes received from the remote server to the memory source
coordinator.addStrategy(
  new SyncStrategy({
    source: "remote",
    target: "memory",
    blocking: true,
  })
);

// Query the remote server whenever the memory source is queried
coordinator.addStrategy(
  new RequestStrategy({
    source: "memory",
    target: "remote",
    on: "beforeQuery",
    action: "query",
    passHints: false,
    blocking: false,
    catch() {
      this.target.requestQueue.skip();
    },
  })
);

// Update the remote server whenever the memory source is updated
coordinator.addStrategy(
  new RequestStrategy({
    source: "memory",
    on: "beforeUpdate",
    target: "remote",
    action: "update",
    passHints: false,
    blocking: false
  })
);

coordinator.addStrategy(
  new RequestStrategy({
    source: "remote",
    on: "updateFail",
    action(transform, e) {
      const source = this.source;
      const store = this.coordinator.getSource("memory");

      // The problem is here, 
      // this if block never gets call when there is network connection problem
      // because the e object is instanceof TypeError and not NetworkError
      if (e instanceof NetworkError) {
        // When network errors are encountered, try again in 3s
        console.log("NetworkError - will try again soon");
        setTimeout(() => {
          source.requestQueue.retry();
        }, 3000);
      } else {
        // When non-network errors occur, notify the user and
        // reset state.
        let label = transform.options && transform.options.label;
        if (label) {
          console.log(`Unable to complete "${label}"`);
        } else {
          console.log(`Unable to complete operation`);
        }

        // Roll back store to position before transform
        if (store.transformLog.contains(transform.id)) {
          console.log("Rolling back - transform:", transform.id);
          store.rollback(transform.id, -1);
        }

        return source.requestQueue.skip();
      }
    }
  })
);

coordinator.addStrategy(
  new RequestStrategy({
    source: "remote",
    on: "queryFail",
    action() {
      this.source.requestQueue.skip();
    },
  })
);

I am using OrbitJS V0.17 and I am following the sample implementation from here https://github.com/orbitjs/todomvc-ember-orbit#scenario-4-memory--backup--remote. Anyone knows why this happened ?

hendra commented 1 year ago

This issue only appear on Chrome, on Firefox the code ran successfully without any issue.

hendra commented 1 year ago

Actually no, it doesn't work either on Firefox.

bradjones1 commented 1 year ago

Each browser's fetch implementation, frustratingly, hints network errors in differetnt ways. Here's a utility class I have, based on the linked code, that helps you sniff for a network error.


export function isNetworkError(error: Error): boolean {
  // From p-retry, but it's not exported.
  // @see https://github.com/danmichaelo/p-retry/blob/6a0e880bb9f15c31a49886775bc5431899f743c5/index.js#L3
  const networkErrorMsgs = new Set([
    'Failed to fetch', // Chrome
    'NetworkError when attempting to fetch resource.', // Firefox
    'The Internet connection appears to be offline.', // Safari
    'Network request failed', // `cross-fetch`
    'fetch failed', // Undici (Node.js)
  ]);
  return networkErrorMsgs.has(error.message);
}
hendra commented 1 year ago

Hi @bradjones1, thanks for the reply.

Actually I have solved the problem similar with yours. I think would be good if this NetworkError handled on the low level instead of the application layer.