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

feat: implement client fallback mechanism #16

Merged
merged 31 commits into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
804978c
feat: implement a nodes list for clients
AmeanAsad Oct 11, 2023
f4d1844
feat: implement a storage interface using indexedDb
AmeanAsad Oct 11, 2023
894d16d
feat: implement a test suite for fallback
AmeanAsad Oct 11, 2023
7009c7a
fix: remove unused code
AmeanAsad Oct 11, 2023
3336a87
fix: eslint an jsdoc
AmeanAsad Oct 11, 2023
696cd8a
fix: formatting and consistency
AmeanAsad Oct 11, 2023
a3dec56
fix: indexDbCheck
AmeanAsad Oct 11, 2023
747d780
Merge branch 'main' into feat/fallback
AmeanAsad Oct 12, 2023
8dec1c7
chore: change storage implementation
AmeanAsad Oct 12, 2023
b0dc673
enhancement: simplify node loading
AmeanAsad Oct 12, 2023
78651fc
naive fallback implementation
AmeanAsad Oct 12, 2023
d15c0ad
modify fallback
AmeanAsad Oct 12, 2023
29e8da1
fix formatting and typos
AmeanAsad Oct 12, 2023
194ce70
typos
AmeanAsad Oct 12, 2023
1d97dc9
Update .eslintrc
AmeanAsad Oct 12, 2023
84a1b8e
enhancement: edit storage impl
AmeanAsad Oct 12, 2023
f688406
enhancement: deal with overlapping byte chunks
AmeanAsad Oct 12, 2023
5b7c215
feat: add fallback test suite
AmeanAsad Oct 14, 2023
6068a90
fix: tests running
AmeanAsad Oct 15, 2023
2c3cc79
cleanup content fetch with fallback
AmeanAsad Oct 16, 2023
4cf0c00
add initial origin fetch to fallback
AmeanAsad Oct 16, 2023
d81038f
formatting and file re-org
AmeanAsad Oct 16, 2023
a1ecd50
feat: merge main into fallback branch (#22)
AmeanAsad Oct 16, 2023
55d56d1
Merge branch 'main' into feat/fallback
AmeanAsad Oct 16, 2023
d2c5c37
load nodes on first success
AmeanAsad Oct 16, 2023
e0065d8
add fallback limit
AmeanAsad Oct 16, 2023
109019b
fix: fallback bug
AmeanAsad Oct 17, 2023
b84ce4a
put eslint settings in package.json
AmeanAsad Oct 17, 2023
7731fe8
add nodesListKey as static
AmeanAsad Oct 17, 2023
e0fcfd8
fix: resolve process in browser
AmeanAsad Oct 18, 2023
ac1b9f8
Merge branch 'main' into feat/fallback
AmeanAsad Oct 19, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,622 changes: 1,515 additions & 107 deletions package-lock.json

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"main": "src/index.js",
"type": "module",
"scripts": {
"test": "NODE_ENV=development node --test test/",
"test": "NODE_ENV=development TESTING=true node --test test/",
"build": "webpack --mode production",
"prepack": "npm run build"
},
Expand All @@ -30,6 +30,7 @@
"browser-readablestream-to-it": "^2.0.4",
"idb": "^7.1.1",
"ipfs-unixfs-exporter": "https://gitpkg.now.sh/filecoin-saturn/js-ipfs-unixfs/packages/ipfs-unixfs-exporter?build",
"msw": "^1.3.2",
"multiformats": "^12.1.1"
},
"devDependencies": {
Expand All @@ -41,10 +42,17 @@
"webpack-cli": "^4.10.0"
},
"eslintConfig": {
"plugins": [
"jsdoc",
"promise"
],
"extends": "ipfs",
"parserOptions": {
"ecmaVersion": "latest",
"sourceType": "module"
"sourceType": "module",
"ecmaVersion": "latest"
},
"rules": {
"jsdoc/no-undefined-types": 1
}
},
"imports": {
Expand All @@ -53,4 +61,4 @@
"publishConfig": {
"access": "public"
}
}
}
133 changes: 124 additions & 9 deletions src/client.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
// @ts-check

import { CID } from 'multiformats'

import { extractVerifiedContent } from './utils/car.js'
import { asAsyncIterable, asyncIteratorToBuffer } from './utils/itr.js'
import { randomUUID } from './utils/uuid.js'
import { memoryStorage } from './storage/index.js'
import { getJWT } from './utils/jwt.js'
import { parseUrl, addHttpPrefix } from './utils/url.js'
import { isBrowserContext } from './utils/runtime.js'

export class Saturn {
static nodesListKey = 'saturn-nodes'
/**
*
* @param {object} [opts={}]
Expand All @@ -16,14 +20,18 @@ export class Saturn {
* @param {string} [opts.cdnURL=saturn.ms]
* @param {number} [opts.connectTimeout=5000]
* @param {number} [opts.downloadTimeout=0]
* @param {string} [opts.orchURL]
* @param {number} [opts.fallbackLimit]
* @param {import('./storage/index.js').Storage} [opts.storage]
*/
constructor (opts = {}) {
this.opts = Object.assign({}, {
clientId: randomUUID(),
cdnURL: 'l1s.saturn.ms',
logURL: 'https://twb3qukm2i654i3tnvx36char40aymqq.lambda-url.us-west-2.on.aws/',
orchURL: 'https://orchestrator.strn.pl/nodes?maxNodes=100',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

@guanzo guanzo Oct 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes thats correct.

authURL: 'https://fz3dyeyxmebszwhuiky7vggmsu0rlkoy.lambda-url.us-west-2.on.aws/',
fallbackLimit: 5,
connectTimeout: 5_000,
downloadTimeout: 0
}, opts)
Expand All @@ -33,13 +41,14 @@ export class Saturn {
}

this.logs = []
this.storage = this.opts.storage || memoryStorage()
this.nodes = []
this.reportingLogs = process?.env?.NODE_ENV !== 'development'
this.hasPerformanceAPI = isBrowserContext && self?.performance

if (this.reportingLogs && this.hasPerformanceAPI) {
this._monitorPerformanceBuffer()
}
this.storage = this.opts.storage || memoryStorage()
this.loadNodesPromise = this._loadNodes(this.opts)
}

/**
Expand Down Expand Up @@ -76,10 +85,9 @@ export class Saturn {
Authorization: 'Bearer ' + options.jwt
}
}

let res
try {
res = await fetch(url, { signal: controller.signal, ...options })
res = await fetch(parseUrl(url), { signal: controller.signal, ...options })

clearTimeout(connectTimeout)

Expand Down Expand Up @@ -109,6 +117,74 @@ export class Saturn {
return { res, controller, log }
}

/**
*
* @param {string} cidPath
* @param {object} [opts={}]
* @param {('car'|'raw')} [opts.format]
* @param {string} [opts.url]
* @param {number} [opts.connectTimeout=5000]
* @param {number} [opts.downloadTimeout=0]
* @returns {Promise<AsyncIterable<Uint8Array>>}
*/
async * fetchContentWithFallback (cidPath, opts = {}) {
let lastError = null
// we use this to checkpoint at which chunk a request failed.
// this is temporary until range requests are supported.
let byteCountCheckpoint = 0

const fetchContent = async function * () {
let byteCount = 0
const byteChunks = await this.fetchContent(cidPath, opts)
for await (const chunk of byteChunks) {
// avoid sending duplicate chunks
if (byteCount < byteCountCheckpoint) {
// checks for overlapping chunks
const remainingBytes = byteCountCheckpoint - byteCount
if (remainingBytes < chunk.length) {
yield chunk.slice(remainingBytes)
AmeanAsad marked this conversation as resolved.
Show resolved Hide resolved
byteCountCheckpoint += chunk.length - remainingBytes
}
} else {
yield chunk
byteCountCheckpoint += chunk.length
}
byteCount += chunk.length
}
}.bind(this)

if (this.nodes.length === 0) {
// fetch from origin in the case that no nodes are loaded
opts.url = this.opts.cdnURL
try {
yield * fetchContent()
return
} catch (err) {
lastError = err
await this.loadNodesPromise
}
}

let fallbackCount = 0
for (const origin of this.nodes) {
if (fallbackCount > this.opts.fallbackLimit) {
return
}
opts.url = origin.url
try {
yield * fetchContent()
return
} catch (err) {
lastError = err
}
fallbackCount += 1
}

if (lastError) {
throw new Error(`All attempts to fetch content have failed. Last error: ${lastError.message}`)
}
}

/**
*
* @param {string} cidPath
Expand Down Expand Up @@ -163,10 +239,8 @@ export class Saturn {
* @returns {URL}
*/
createRequestURL (cidPath, opts) {
let origin = opts.cdnURL
if (!origin.startsWith('http')) {
origin = `https://${origin}`
}
let origin = opts.url || opts.cdnURL
origin = addHttpPrefix(origin)
const url = new URL(`${origin}/ipfs/${cidPath}`)

url.searchParams.set('format', opts.format)
Expand Down Expand Up @@ -196,7 +270,6 @@ export class Saturn {
*/
reportLogs (log) {
if (!this.reportingLogs) return

this.logs.push(log)
this.reportLogsTimeout && clearTimeout(this.reportLogsTimeout)
this.reportLogsTimeout = setTimeout(this._reportLogs.bind(this), 3_000)
Expand Down Expand Up @@ -295,4 +368,46 @@ export class Saturn {
performance.clearResourceTimings()
}
}

async _loadNodes (opts) {
let origin = opts.orchURL

let cacheNodesListPromise
if (this.storage) {
cacheNodesListPromise = this.storage.get(Saturn.nodesListKey)
}

origin = addHttpPrefix(origin)

const url = new URL(origin)
const controller = new AbortController()
const options = Object.assign({}, { method: 'GET' }, this.opts)

const connectTimeout = setTimeout(() => {
controller.abort()
}, options.connectTimeout)

const orchResponse = await fetch(parseUrl(url), { signal: controller.signal, ...options })
const orchNodesListPromise = orchResponse.json()
clearTimeout(connectTimeout)

// This promise races fetching nodes list from the orchestrator and
// and the provided storage object (localStorage, sessionStorage, etc.)
// to insure we have a fallback set as quick as possible
let nodes
if (cacheNodesListPromise) {
nodes = await Promise.any([orchNodesListPromise, cacheNodesListPromise])
} else {
nodes = await orchNodesListPromise
}

// if storage returns first, update based on cached storage.
if (nodes === await cacheNodesListPromise) {
this.nodes = nodes
}
// we always want to update from the orchestrator regardless.
nodes = await orchNodesListPromise
this.nodes = nodes
this.storage?.set(Saturn.nodesListKey, nodes)
}
}
32 changes: 32 additions & 0 deletions src/utils/url.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// @ts-check

/**
*
* @param {URL} url
* @returns {URL|string}
*/
export function parseUrl (url) {
try {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// This is a temp function to resolve URLs for mock testing
// See issue here: https://github.com/mswjs/msw/issues/1597
if (process?.env?.TESTING) {
return url.toJSON()
}
} catch (e) {}

return url
}

/**
*
* @param {string} url
* @returns {string}
*/
export function addHttpPrefix (url) {
// This is a temp function to resolve URLs for mock testing
// See issue here: https://github.com/mswjs/msw/issues/1597
if (!url.startsWith('http')) {
url = `https://${url}`
}
return url
}
20 changes: 2 additions & 18 deletions test/car.spec.js
Original file line number Diff line number Diff line change
@@ -1,28 +1,13 @@
import assert from 'node:assert/strict'
import fs from 'node:fs'
import { describe, it } from 'node:test'
import { resolve, dirname } from 'node:path'
import { fileURLToPath } from 'node:url'
import { getFixturePath, concatChunks } from './test-utils.js'

import { CarReader, CarWriter } from '@ipld/car'
import { CID } from 'multiformats/cid'

import { extractVerifiedContent } from '#src/utils/car.js'

const __dirname = dirname(fileURLToPath(import.meta.url))

function getFixturePath (filename) {
return resolve(__dirname, `./fixtures/${filename}`)
}

async function concatChunks (itr) {
const arr = []
for await (const chunk of itr) {
arr.push(chunk)
}
return new Uint8Array(...arr)
}

describe('CAR Verification', () => {
it('should extract content from a valid CAR', async () => {
const cidPath =
Expand Down Expand Up @@ -149,8 +134,7 @@ describe('CAR Verification', () => {

await assert.rejects(
async () => {
for await (const _ of extractVerifiedContent(cidPath, out)) {
}
for await (const _ of extractVerifiedContent(cidPath, out)) {}
},
{
name: 'VerificationError',
Expand Down
Loading