Skip to content

Commit

Permalink
feat: dont use ipfs-utils fetch with upload progress by default (#1240)
Browse files Browse the repository at this point in the history
because importing it in bundled setups breaks, e.g.
* #1183
* #828

I tested this change locally and confirmed that once it's made,
upload-client can be relied on by a nextjs project and build
successfully
  • Loading branch information
gobengo authored Dec 13, 2023
1 parent 468bb79 commit 86aedbc
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 14 deletions.
1 change: 1 addition & 0 deletions packages/upload-client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
8 changes: 8 additions & 0 deletions packages/upload-client/src/fetch-with-upload-progress.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import ipfsUtilsFetch from 'ipfs-utils/src/http/fetch.js'

/**
* @type {import('./types.js').FetchWithUploadProgress}
*/
export const fetchWithUploadProgress = (url, init) => {
return ipfsUtilsFetch.fetch(url, init)
}
27 changes: 18 additions & 9 deletions packages/upload-client/src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
*
Expand Down Expand Up @@ -90,10 +88,9 @@ export async function add(
const responseAddUpload = result.out.ok

const fetchWithUploadProgress =
/** @type {(url: string, init?: import('./types.js').FetchOptions) => Promise<Response>} */ (
fetch
)
options.fetchWithUploadProgress || options.fetch || globalThis.fetch

let fetchDidCallUploadProgressCb = false
const res = await retry(
async () => {
try {
Expand All @@ -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',
})
Expand All @@ -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}`)
}
Expand Down
22 changes: 20 additions & 2 deletions packages/upload-client/src/types.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -45,6 +45,16 @@ import {
UsageReportFailure,
} from '@web3-storage/capabilities/types'

type Override<T, R> = Omit<T, keyof R> & R

type FetchOptions = Override<
IpfsUtilsFetchOptions,
{
// `fetch` is a browser API and browsers don't have `Readable`
body: Exclude<IpfsUtilsFetchOptions['body'], import('node:stream').Readable>
}
>

export type {
FetchOptions,
StoreAdd,
Expand Down Expand Up @@ -186,7 +196,13 @@ export interface Connectable {
connection?: ConnectionView<Service>
}

export type FetchWithUploadProgress = (
url: string,
init?: FetchOptions
) => Promise<Response>

export interface UploadProgressTrackable {
fetchWithUploadProgress?: FetchWithUploadProgress
onUploadProgress?: ProgressFn
}

Expand All @@ -210,7 +226,9 @@ export interface RequestOptions
extends Retryable,
Abortable,
Connectable,
UploadProgressTrackable {}
UploadProgressTrackable {
fetch?: typeof globalThis.fetch
}

export interface ListRequestOptions extends RequestOptions, Pageable {}

Expand Down
34 changes: 31 additions & 3 deletions packages/upload-client/test/store.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -62,25 +63,52 @@ 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,
{
connection,
onUploadProgress: (status) => {
assert(typeof status.loaded === 'number' && status.loaded > 0)
loaded = status.loaded
progress.push(status)
},
fetchWithUploadProgress,
}
)

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 () => {
Expand Down

0 comments on commit 86aedbc

Please sign in to comment.