From 1f9189f10caa36e151eac19e6c0c54906e2e5dd2 Mon Sep 17 00:00:00 2001 From: bengo <171782+gobengo@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:16:48 -0800 Subject: [PATCH 1/4] dont use ipfs-utils fetch with upload progress by default because importing it in bundled setups breaks --- .../src/fetch-with-upload-progress.js | 11 ++++++++ packages/upload-client/src/store.js | 27 ++++++++++++------- packages/upload-client/src/types.ts | 22 +++++++++++++-- packages/upload-client/test/store.test.js | 2 ++ 4 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 packages/upload-client/src/fetch-with-upload-progress.js diff --git a/packages/upload-client/src/fetch-with-upload-progress.js b/packages/upload-client/src/fetch-with-upload-progress.js new file mode 100644 index 000000000..4d6ef1f7b --- /dev/null +++ b/packages/upload-client/src/fetch-with-upload-progress.js @@ -0,0 +1,11 @@ +import ipfsUtilsFetch from 'ipfs-utils/src/http/fetch.js' + +/** + * @type {import('./types.js').FetchWithUploadProgress} + */ +export const fetchWithUploadProgress = (url, init) => { + if (init && 'readable' in init) { + throw new Error('init cannot be readable') + } + return ipfsUtilsFetch.fetch(url, init) +} diff --git a/packages/upload-client/src/store.js b/packages/upload-client/src/store.js index d92b906a6..764c76238 100644 --- a/packages/upload-client/src/store.js +++ b/packages/upload-client/src/store.js @@ -4,8 +4,6 @@ import { SpaceDID } from '@web3-storage/capabilities/utils' import retry, { AbortError } from 'p-retry' import { servicePrincipal, connection } from './service.js' import { REQUEST_RETRIES } from './constants.js' -import fetchPkg from 'ipfs-utils/src/http/fetch.js' -const { fetch } = fetchPkg /** * @@ -90,10 +88,9 @@ export async function add( const responseAddUpload = result.out.ok const fetchWithUploadProgress = - /** @type {(url: string, init?: import('./types.js').FetchOptions) => Promise} */ ( - fetch - ) + options.fetchWithUploadProgress || options.fetch || globalThis.fetch + let fetchDidCallUploadProgressCb = false const res = await retry( async () => { try { @@ -103,12 +100,14 @@ export async function add( body: car, headers: responseAddUpload.headers, signal: options.signal, - onUploadProgress: options.onUploadProgress - ? createUploadProgressHandler( + onUploadProgress: (status) => { + fetchDidCallUploadProgressCb = true + if (options.onUploadProgress) + createUploadProgressHandler( responseAddUpload.url, options.onUploadProgress - ) - : undefined, + )(status) + }, // @ts-expect-error - this is needed by recent versions of node - see https://github.com/bluesky-social/atproto/pull/470 for more info duplex: 'half', }) @@ -128,6 +127,16 @@ export async function add( } ) + if (!fetchDidCallUploadProgressCb && options.onUploadProgress) { + // the fetch implementation didn't support onUploadProgress + const carBlob = new Blob([car]) + options.onUploadProgress({ + total: carBlob.size, + loaded: carBlob.size, + lengthComputable: false, + }) + } + if (!res.ok) { throw new Error(`upload failed: ${res.status}`) } diff --git a/packages/upload-client/src/types.ts b/packages/upload-client/src/types.ts index 582bb114d..b80a7cd5e 100644 --- a/packages/upload-client/src/types.ts +++ b/packages/upload-client/src/types.ts @@ -1,5 +1,5 @@ import type { - FetchOptions, + FetchOptions as IpfsUtilsFetchOptions, ProgressStatus as XHRProgressStatus, } from 'ipfs-utils/src/types.js' import { Link, UnknownLink, Version } from 'multiformats/link' @@ -45,6 +45,16 @@ import { UsageReportFailure, } from '@web3-storage/capabilities/types' +type Override = Omit & R + +type FetchOptions = Override< + IpfsUtilsFetchOptions, + { + // `fetch` is a browser API and browsers don't have `Readable` + body: Exclude + } +> + export type { FetchOptions, StoreAdd, @@ -186,7 +196,13 @@ export interface Connectable { connection?: ConnectionView } +export type FetchWithUploadProgress = ( + url: string, + init?: FetchOptions +) => Promise + export interface UploadProgressTrackable { + fetchWithUploadProgress?: FetchWithUploadProgress onUploadProgress?: ProgressFn } @@ -210,7 +226,9 @@ export interface RequestOptions extends Retryable, Abortable, Connectable, - UploadProgressTrackable {} + UploadProgressTrackable { + fetch?: typeof globalThis.fetch +} export interface ListRequestOptions extends RequestOptions, Pageable {} diff --git a/packages/upload-client/test/store.test.js b/packages/upload-client/test/store.test.js index 2f53d5f8c..376daf2f2 100644 --- a/packages/upload-client/test/store.test.js +++ b/packages/upload-client/test/store.test.js @@ -10,6 +10,7 @@ import { serviceSigner } from './fixtures.js' import { randomCAR } from './helpers/random.js' import { mockService } from './helpers/mocks.js' import { validateAuthorization } from './helpers/utils.js' +import { fetchWithUploadProgress } from '../src/fetch-with-upload-progress.js' describe('Store.add', () => { it('stores a DAG with the service', async () => { @@ -71,6 +72,7 @@ describe('Store.add', () => { assert(typeof status.loaded === 'number' && status.loaded > 0) loaded = status.loaded }, + fetchWithUploadProgress, } ) From 6d78127430a49167675d96631be2db0ce820c73d Mon Sep 17 00:00:00 2001 From: bengo <171782+gobengo@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:20:11 -0800 Subject: [PATCH 2/4] rm unused logic --- packages/upload-client/src/fetch-with-upload-progress.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/upload-client/src/fetch-with-upload-progress.js b/packages/upload-client/src/fetch-with-upload-progress.js index 4d6ef1f7b..f33087c8f 100644 --- a/packages/upload-client/src/fetch-with-upload-progress.js +++ b/packages/upload-client/src/fetch-with-upload-progress.js @@ -4,8 +4,5 @@ import ipfsUtilsFetch from 'ipfs-utils/src/http/fetch.js' * @type {import('./types.js').FetchWithUploadProgress} */ export const fetchWithUploadProgress = (url, init) => { - if (init && 'readable' in init) { - throw new Error('init cannot be readable') - } return ipfsUtilsFetch.fetch(url, init) } From e845eb9ed19430eb67ffac0a47d1bf5009537ab4 Mon Sep 17 00:00:00 2001 From: bengo <171782+gobengo@users.noreply.github.com> Date: Wed, 13 Dec 2023 10:23:42 -0800 Subject: [PATCH 3/4] add export fetch-with-upload-progress --- packages/upload-client/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/upload-client/package.json b/packages/upload-client/package.json index 21711085a..810fc2eed 100644 --- a/packages/upload-client/package.json +++ b/packages/upload-client/package.json @@ -31,6 +31,7 @@ "exports": { ".": "./dist/src/index.js", "./car": "./dist/src/car.js", + "./fetch-with-upload-progress": "./dist/src/fetch-with-upload-progress.js", "./sharding": "./dist/src/sharding.js", "./upload": "./dist/src/upload.js", "./store": "./dist/src/store.js", From 403356d3e2003672aa46b3c1275907ff15d9026f Mon Sep 17 00:00:00 2001 From: bengo <171782+gobengo@users.noreply.github.com> Date: Wed, 13 Dec 2023 11:02:15 -0800 Subject: [PATCH 4/4] fix coverage by testing Store.add sans fetchWithUploadProgress --- packages/upload-client/test/store.test.js | 32 ++++++++++++++++++++--- 1 file changed, 29 insertions(+), 3 deletions(-) diff --git a/packages/upload-client/test/store.test.js b/packages/upload-client/test/store.test.js index c36411ee6..90574ef8b 100644 --- a/packages/upload-client/test/store.test.js +++ b/packages/upload-client/test/store.test.js @@ -63,7 +63,8 @@ describe('Store.add', () => { channel: server, }) - let loaded = 0 + /** @type {import('../src/types.js').ProgressStatus[]} */ + const progress = [] const carCID = await Store.add( { issuer: agent, with: space.did(), proofs, audience: serviceSigner }, car, @@ -71,7 +72,7 @@ describe('Store.add', () => { connection, onUploadProgress: (status) => { assert(typeof status.loaded === 'number' && status.loaded > 0) - loaded = status.loaded + progress.push(status) }, fetchWithUploadProgress, } @@ -79,10 +80,35 @@ describe('Store.add', () => { assert(service.store.add.called) assert.equal(service.store.add.callCount, 1) - assert.equal(loaded, 225) + assert.equal( + progress.reduce((max, { loaded }) => Math.max(max, loaded), 0), + 225 + ) assert(carCID) assert.equal(carCID.toString(), car.cid.toString()) + + // make sure it can also work without fetchWithUploadProgress + /** @type {import('../src/types.js').ProgressStatus[]} */ + let progressWithoutUploadProgress = [] + const addedWithoutUploadProgress = await Store.add( + { issuer: agent, with: space.did(), proofs, audience: serviceSigner }, + car, + { + connection, + onUploadProgress: (status) => { + progressWithoutUploadProgress.push(status) + }, + } + ) + assert.equal(addedWithoutUploadProgress.toString(), car.cid.toString()) + assert.equal( + progressWithoutUploadProgress.reduce( + (max, { loaded }) => Math.max(max, loaded), + 0 + ), + 225 + ) }) it('throws for bucket URL client error 4xx', async () => {