feat: implement client fallback mechanism #16

AmeanAsad · 2023-10-11T04:18:46Z

Changes:

Adding a managed nodes list for the client:

Introduces a local nodes array that the client manages.
Adds a storage option to the client which can be injected as long as it matches the defined Storage interface.
Implements a Storage implementation that uses IndexedDb. This will be the first storage implementation as this is the only one that service workers support immediately. More can be added as needed.
The logic to load nodes races the orchestrator and the provided storage option. Which ever responds first sets the nodes list. This is to ensure we set the fall back as quick as possible. The list still updates with the latest orch nodes afterwards regardless.

Implementing a test suite:

Added a setup for mocking the orchestrator and mocking individual L1 nodes using the wsj library. This allows for more robust testing locally.
Added a comprehensive test suite that tests the logic of the fallback code added. This assumes that the provided Storage and the Orchestrator components work as intended.

src/index.js

src/utils/storage.js

AmeanAsad · 2023-10-11T15:52:13Z

@guanzo also enabled eslint + jsdoc type checking

src/index.js

guanzo · 2023-10-12T16:15:51Z

src/index.js

+    // we use this to checkpoint at which chunk a request failed.
+    // this is temporary until range requests are supported.
+    let byteCountCheckpoint = 0
+    for (const origin of this.nodes) {


Retrying 100 times seems a bit overkill, especially if the error is consistent and will throw on every attempt. Like there's something wrong with the DAG itself and not the L1. I think we should start smaller at like 10 and see how it goes.

yeah agreed, was thinking about that as well.

Wondering if there are errors that we should handle, like if the js client verification fails due to some js error then it will probably fail for every fallback node regardless

yeah there'll be a class of errors where we should just bail instead of retrying. i think "file not found" is one of them.

I've added a limit to the amount we can fallback. I do think some errors should be bailed right away but i think id save that for another PR.

Im also worried about perf with error matching mid request. Thoughts?

Perf shouldn't be an issue unless the error list is huge, like 10k+.

src/index.js

.eslintrc

Co-authored-by: Diego Rodríguez Baquero <diego@protocol.ai>

* Abort on error (#19) * feat: use controller from options if exists. abort fetch if error occurs. * test: check if external abort controller is used * build: move build output to dist/ folder * fix: newline * 0.1.1 * Build exports (#20) * chore: rename file * feat: add new entrypoint with exports. Switch Saturn to named export * build: expose entire module instead of just the default export * docs: update README * 0.2.0 * feat: include worker scopes when checking for browser runtime (#21) * 0.3.0 --------- Co-authored-by: Eric Guan <guanzo91@gmail.com>

AmeanAsad · 2023-10-16T14:28:44Z

src/client.js

   * @param {import('./storage/index.js').Storage} [opts.storage]
   */
  constructor (opts = {}) {
    this.opts = Object.assign({}, {
      clientId: randomUUID(),
      cdnURL: 'saturn.ms',
      logURL: 'https://twb3qukm2i654i3tnvx36char40aymqq.lambda-url.us-west-2.on.aws/',
+      orchURL: 'https://orchestrator.strn.pl/nodes?maxNodes=100',


@guanzo I want to clean this up a little by having those URLs in one place and they get imported here. You cool with that?

agree on this if possible -- having these really weird URLs hardcoded right in the code is a bit odd. I might this a private static class readonly var "defaultConfig" or something

Do we want to keep our auth URL at least a private var?

Doesn't completely hide it but at least makes it harder for someone to find it and use it.

These kinds of URL config vars should typically be env vars injected during the build process.

When you guys say "private var" do you mean https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields?

yes thats correct.

hannahhoward

generally LGTM. and overall, awsome PR.

everything is a suggestion but I do think you have a minor bug in FetchContentWithFallback when there is byte overlap. Definitely respond to my comment there before you merge

hannahhoward · 2023-10-16T17:21:16Z

.eslintrc

@@ -0,0 +1,14 @@
+{


Suggestion: if we're going to have this, we should remove the config from package.json: https://github.com/filecoin-saturn/js-client/pull/16/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L43

I just added the config to the package.json. Didn't notice it was there in the first place, so I just removed the .eslintrc and ported over the updated config.

hannahhoward · 2023-10-16T17:23:58Z

src/client.js

   * @param {import('./storage/index.js').Storage} [opts.storage]
   */
  constructor (opts = {}) {
    this.opts = Object.assign({}, {
      clientId: randomUUID(),
      cdnURL: 'saturn.ms',
      logURL: 'https://twb3qukm2i654i3tnvx36char40aymqq.lambda-url.us-west-2.on.aws/',
+      orchURL: 'https://orchestrator.strn.pl/nodes?maxNodes=100',


agree on this if possible -- having these really weird URLs hardcoded right in the code is a bit odd. I might this a private static class readonly var "defaultConfig" or something

hannahhoward · 2023-10-16T17:25:52Z

src/client.js

@@ -33,13 +40,15 @@ export class Saturn {
    }

    this.logs = []
-    this.storage = this.opts.storage || memoryStorage()
+    this.nodes = []
+    this.nodesListKey = 'saturn-nodes'


is this ever going to change? if not, I'd again just make it a static readonly var.

test/fallback.spec.js

src/client.js

test/test-utils.js

AmeanAsad · 2023-10-18T20:08:52Z

src/utils/url.js

+ * @returns {URL|string}
+ */
+export function parseUrl (url) {
+  try {


@guanzo had to add this try catch here because when testing in the browser, process is not defined. Seems like webpack's environment plugin doesn't make process accessible in libraries that you import from. This parseUrl function is only for testing though and is temporary so think this is fine for now.

why's it being imported by the browser-client if its only used for testing

sorry its not only for testing, parseUrl is used within the js-client code. Im was just saying the reason it exists because the mock tests don't accept URL objects in fetch so had to add this to make them run.

AmeanAsad added 3 commits October 11, 2023 00:17

feat: implement a nodes list for clients

804978c

feat: implement a storage interface using indexedDb

f4d1844

feat: implement a test suite for fallback

894d16d

AmeanAsad requested a review from guanzo October 11, 2023 04:24

AmeanAsad added 2 commits October 11, 2023 10:27

fix: remove unused code

7009c7a

fix: eslint an jsdoc

3336a87

guanzo reviewed Oct 11, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

fix: formatting and consistency

696cd8a

guanzo reviewed Oct 11, 2023

View reviewed changes

src/utils/storage.js Outdated Show resolved Hide resolved

fix: indexDbCheck

a3dec56

guanzo reviewed Oct 11, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

AmeanAsad added 3 commits October 11, 2023 22:34

Merge branch 'main' into feat/fallback

747d780

chore: change storage implementation

8dec1c7

enhancement: simplify node loading

b0dc673

guanzo reviewed Oct 12, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

guanzo reviewed Oct 12, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

AmeanAsad added 4 commits October 12, 2023 11:28

naive fallback implementation

78651fc

modify fallback

d15c0ad

fix formatting and typos

29e8da1

typos

194ce70

guanzo reviewed Oct 12, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

guanzo reviewed Oct 12, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

guanzo reviewed Oct 12, 2023

View reviewed changes

src/index.js Outdated Show resolved Hide resolved

DiegoRBaquero reviewed Oct 12, 2023

View reviewed changes

.eslintrc Outdated Show resolved Hide resolved

AmeanAsad and others added 4 commits October 12, 2023 14:38

Update .eslintrc

1d97dc9

Co-authored-by: Diego Rodríguez Baquero <diego@protocol.ai>

enhancement: edit storage impl

84a1b8e

enhancement: deal with overlapping byte chunks

f688406

feat: add fallback test suite

5b7c215

AmeanAsad and others added 6 commits October 15, 2023 19:44

fix: tests running

6068a90

cleanup content fetch with fallback

2c3cc79

add initial origin fetch to fallback

4cf0c00

formatting and file re-org

d81038f

Merge branch 'main' into feat/fallback

55d56d1

AmeanAsad marked this pull request as ready for review October 16, 2023 14:27

AmeanAsad commented Oct 16, 2023

View reviewed changes

AmeanAsad added 2 commits October 16, 2023 10:34

load nodes on first success

d2c5c37

add fallback limit

e0065d8

hannahhoward reviewed Oct 16, 2023

View reviewed changes

AmeanAsad added 4 commits October 17, 2023 14:25

fix: fallback bug

109019b

put eslint settings in package.json

b84ce4a

add nodesListKey as static

7731fe8

fix: resolve process in browser

e0fcfd8

AmeanAsad commented Oct 18, 2023

View reviewed changes

Merge branch 'main' into feat/fallback

ac1b9f8

guanzo approved these changes Oct 24, 2023

View reviewed changes

AmeanAsad merged commit 15bf85b into main Oct 24, 2023
1 check passed

AmeanAsad deleted the feat/fallback branch October 24, 2023 15:55

AmeanAsad mentioned this pull request Oct 31, 2023

Implement a fallback using the Orchestrator #11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: implement client fallback mechanism #16

feat: implement client fallback mechanism #16

AmeanAsad commented Oct 11, 2023 •

edited

Loading

AmeanAsad commented Oct 11, 2023

guanzo Oct 12, 2023

AmeanAsad Oct 12, 2023

guanzo Oct 12, 2023

AmeanAsad Oct 16, 2023

guanzo Oct 17, 2023

AmeanAsad Oct 16, 2023

hannahhoward Oct 16, 2023

AmeanAsad Oct 17, 2023

guanzo Oct 17, 2023 •

edited

Loading

AmeanAsad Oct 18, 2023

hannahhoward left a comment •

edited

Loading

hannahhoward Oct 16, 2023

AmeanAsad Oct 17, 2023

hannahhoward Oct 16, 2023

hannahhoward Oct 16, 2023

AmeanAsad Oct 18, 2023

guanzo Oct 18, 2023

AmeanAsad Oct 18, 2023

feat: implement client fallback mechanism #16

feat: implement client fallback mechanism #16

Conversation

AmeanAsad commented Oct 11, 2023 • edited Loading

Changes:

AmeanAsad commented Oct 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guanzo Oct 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hannahhoward left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AmeanAsad commented Oct 11, 2023 •

edited

Loading

guanzo Oct 17, 2023 •

edited

Loading

hannahhoward left a comment •

edited

Loading