Skip to content

Commit 9a7a484

Browse files
SgtPookiSgtPooki
authored
fix: upload-flow renders correct spacing and IPNI info (#191)
* tmp * fix: upload-flow renders correct spacing and IPNI info * refactor: rename multiOperationSpinner filename
1 parent ef6adc1 commit 9a7a484

File tree

6 files changed

+281
-101
lines changed

6 files changed

+281
-101
lines changed

‎src/common/upload-flow.ts‎

Lines changed: 59 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import type { AutoFundOptions } from '../payments/types.js'
2424
import type { Spinner } from '../utils/cli-helpers.js'
2525
import { cancel, formatFileSize } from '../utils/cli-helpers.js'
2626
import { log } from '../utils/cli-logger.js'
27+
import { createSpinnerFlow } from '../utils/multi-operation-spinner.js'
2728

2829
export interface UploadFlowOptions {
2930
/**
@@ -249,42 +250,13 @@ export async function performUpload(
249250
): Promise<UploadFlowResult> {
250251
const { contextType, logger, spinner } = options
251252

252-
spinner?.start('Uploading to Filecoin...')
253+
// Create spinner flow manager for tracking all operations
254+
const flow = createSpinnerFlow(spinner)
253255

254-
// Track parallel operations with their messages
255-
const pendingOps = new Map<string, string>()
256-
let transactionHash: string | undefined
257-
258-
function getSpinnerMessage() {
259-
return Array.from(pendingOps.values())
260-
.map((op) => op)
261-
.join(' & ')
262-
}
256+
// Start with upload operation
257+
flow.addOperation('upload', 'Uploading to Filecoin...')
263258

264-
function completeOperation(
265-
operationKey: string,
266-
completionMessage: string,
267-
type: 'success' | 'warning' | 'info' | 'none' = 'success'
268-
) {
269-
pendingOps.delete(operationKey)
270-
271-
switch (type) {
272-
case 'success':
273-
spinner?.stop(`${pc.green('✓')} ${completionMessage}`)
274-
break
275-
case 'warning':
276-
spinner?.stop(`${pc.yellow('⚠')} ${completionMessage}`)
277-
break
278-
default:
279-
spinner?.stop(completionMessage)
280-
break
281-
}
282-
283-
// Restart spinner with remaining operations if any
284-
if (pendingOps.size > 0) {
285-
spinner?.start(getSpinnerMessage())
286-
}
287-
}
259+
let transactionHash: string | undefined
288260

289261
let pieceCid: PieceCID | undefined
290262
function getIpniAdvertisementMsg(attemptCount: number): string {
@@ -298,69 +270,78 @@ export async function performUpload(
298270
switch (event.type) {
299271
case 'onUploadComplete': {
300272
pieceCid = event.data.pieceCid
301-
spinner?.stop(`${pc.green('✓')} Upload complete`)
302-
const serviceURL = getServiceURL(synapseService.providerInfo)
303-
if (serviceURL != null && serviceURL !== '') {
304-
log.spinnerSection('Download IPFS CAR from SP', [
305-
pc.gray(`${serviceURL.replace(/\/$/, '')}/ipfs/${rootCid}`),
306-
])
307-
}
308-
spinner?.start('Adding piece to DataSet...')
273+
flow.completeOperation('upload', 'Upload complete', {
274+
type: 'success',
275+
details: (() => {
276+
const serviceURL = getServiceURL(synapseService.providerInfo)
277+
if (serviceURL != null && serviceURL !== '') {
278+
return {
279+
title: 'Download IPFS CAR from SP',
280+
content: [pc.gray(`${serviceURL.replace(/\/$/, '')}/ipfs/${rootCid}`)],
281+
}
282+
}
283+
return
284+
})(),
285+
})
286+
// Start adding piece to dataset operation
287+
flow.addOperation('add-to-dataset', 'Adding piece to DataSet...')
309288
break
310289
}
311290
case 'onPieceAdded': {
312-
spinner?.stop(`${pc.green('✓')} Piece added to DataSet (unconfirmed on-chain)`)
313291
if (event.data.txHash) {
314292
transactionHash = event.data.txHash
315293
}
316-
log.spinnerSection('Explorer URLs', [
317-
pc.gray(`Piece: https://pdp.vxb.ai/calibration/piece/${pieceCid}`),
318-
pc.gray(
319-
`Transaction: https://${synapseService.synapse.getNetwork()}.filfox.info/en/message/${transactionHash}`
320-
),
321-
])
322-
323-
pendingOps.set('chain', 'Confirming piece added to DataSet on-chain')
324-
325-
spinner?.start(getSpinnerMessage())
294+
flow.completeOperation('add-to-dataset', 'Piece added to DataSet (unconfirmed on-chain)', {
295+
type: 'success',
296+
details: {
297+
title: 'Explorer URLs',
298+
content: [
299+
pc.gray(`Piece: https://pdp.vxb.ai/calibration/piece/${pieceCid}`),
300+
pc.gray(
301+
`Transaction: https://${synapseService.synapse.getNetwork()}.filfox.info/en/message/${transactionHash}`
302+
),
303+
],
304+
},
305+
})
306+
// Start chain confirmation operation
307+
flow.addOperation('chain', 'Confirming piece added to DataSet on-chain')
326308
break
327309
}
328310
case 'onPieceConfirmed': {
329-
completeOperation('chain', `Piece added to DataSet (confirmed on-chain)`, 'success')
311+
flow.completeOperation('chain', 'Piece added to DataSet (confirmed on-chain)', {
312+
type: 'success',
313+
})
330314
break
331315
}
332316

333317
case 'ipniAdvertisement.retryUpdate': {
334-
if (event.data.retryCount === 0) {
335-
pendingOps.set('ipni', getIpniAdvertisementMsg(1))
336-
}
337-
pendingOps.set('ipni', getIpniAdvertisementMsg(event.data.retryCount + 1))
338-
spinner?.message(getSpinnerMessage())
318+
const attemptCount = event.data.retryCount === 0 ? 1 : event.data.retryCount + 1
319+
flow.addOperation('ipni', getIpniAdvertisementMsg(attemptCount))
339320
break
340321
}
341322
case 'ipniAdvertisement.complete': {
342-
const isIpniAdvertisementSuccessful = event.data.result
343-
const message = isIpniAdvertisementSuccessful
344-
? `IPNI advertisement successful. IPFS retrieval possible.`
345-
: `IPNI advertisement pending`
346-
347-
completeOperation('ipni', message, isIpniAdvertisementSuccessful ? 'success' : 'warning')
348-
349-
if (isIpniAdvertisementSuccessful) {
350-
log.spinnerSection('IPFS Retrieval URLs', [
351-
pc.gray(`ipfs://${rootCid}`),
352-
pc.gray(`https://inbrowser.link/ipfs/${rootCid}`),
353-
pc.gray(`https://dweb.link/ipfs/${rootCid}`),
354-
])
355-
}
323+
// complete event is only emitted when result === true (success)
324+
flow.completeOperation('ipni', 'IPNI advertisement successful. IPFS retrieval possible.', {
325+
type: 'success',
326+
details: {
327+
title: 'IPFS Retrieval URLs',
328+
content: [
329+
pc.gray(`ipfs://${rootCid}`),
330+
pc.gray(`https://inbrowser.link/ipfs/${rootCid}`),
331+
pc.gray(`https://dweb.link/ipfs/${rootCid}`),
332+
],
333+
},
334+
})
356335
break
357336
}
358337
case 'ipniAdvertisement.failed': {
359-
logger.error({ error: event.data.error }, 'Error checking IPNI advertisement')
360-
completeOperation('ipni', `IPNI advertisement check failed`, 'warning')
361-
log.spinnerSection('IPNI advertisement check failed', [
362-
pc.gray(`IPNI advertisement does not exist at http://filecoinpin.contact/cid/${rootCid}`),
363-
])
338+
flow.completeOperation('ipni', 'IPNI advertisement failed.', {
339+
type: 'warning',
340+
details: {
341+
title: 'IPFS retrieval is not possible yet.',
342+
content: [pc.gray(`IPNI advertisement does not exist at http://filecoinpin.contact/cid/${rootCid}`)],
343+
},
344+
})
364345
break
365346
}
366347
default: {

‎src/core/upload/index.ts‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -230,16 +230,15 @@ export async function executeUpload(
230230
case 'onPieceAdded': {
231231
// Begin IPNI validation as soon as the piece is added and parked in the data set
232232
if (options.ipniValidation?.enabled !== false && ipniValidationPromise == null) {
233-
try {
234-
const { enabled: _enabled, ...rest } = options.ipniValidation ?? {}
235-
ipniValidationPromise = validateIPNIAdvertisement(rootCid, {
236-
...rest,
237-
logger,
238-
})
239-
} catch (error) {
240-
logger.error({ error }, 'Could not begin IPNI advertisement validation')
241-
ipniValidationPromise = Promise.resolve(false)
242-
}
233+
const { enabled: _enabled, ...rest } = options.ipniValidation ?? {}
234+
ipniValidationPromise = validateIPNIAdvertisement(rootCid, {
235+
...rest,
236+
logger,
237+
...(options?.onProgress != null ? { onProgress: options.onProgress } : {}),
238+
}).catch((error) => {
239+
logger.warn({ error }, 'IPNI advertisement validation promise rejected')
240+
return false
241+
})
243242
}
244243
if (event.data.txHash != null) {
245244
transactionHash = event.data.txHash

‎src/core/utils/validate-ipni-advertisement.ts‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { ProgressEvent, ProgressEventHandler } from './types.js'
44

55
export type ValidateIPNIProgressEvents =
66
| ProgressEvent<'ipniAdvertisement.retryUpdate', { retryCount: number }>
7-
| ProgressEvent<'ipniAdvertisement.complete', { result: boolean; retryCount: number }>
7+
| ProgressEvent<'ipniAdvertisement.complete', { result: true; retryCount: number }>
88
| ProgressEvent<'ipniAdvertisement.failed', { error: Error }>
99

1010
export interface ValidateIPNIAdvertisementOptions {
@@ -81,19 +81,20 @@ export async function validateIPNIAdvertisement(
8181
try {
8282
options?.onProgress?.({ type: 'ipniAdvertisement.retryUpdate', data: { retryCount } })
8383
} catch (error) {
84-
options?.logger?.error({ error }, 'Error in consumer onProgress callback for retryUpdate event')
84+
options?.logger?.warn({ error }, 'Error in consumer onProgress callback for retryUpdate event')
8585
}
8686

8787
const response = await fetch(`https://filecoinpin.contact/cid/${ipfsRootCid}`, fetchOptions)
8888
if (response.ok) {
8989
try {
9090
options?.onProgress?.({ type: 'ipniAdvertisement.complete', data: { result: true, retryCount } })
9191
} catch (error) {
92-
options?.logger?.error({ error }, 'Error in consumer onProgress callback for complete event')
92+
options?.logger?.warn({ error }, 'Error in consumer onProgress callback for complete event')
9393
}
9494
resolve(true)
9595
return
9696
}
97+
9798
if (++retryCount < maxAttempts) {
9899
options?.logger?.info(
99100
{ retryCount, maxAttempts },
@@ -106,20 +107,21 @@ export async function validateIPNIAdvertisement(
106107
await new Promise((resolve) => setTimeout(resolve, delayMs))
107108
await check()
108109
} else {
110+
// Max attempts reached - don't emit 'complete' event, just throw
111+
// The outer catch handler will emit 'failed' event
109112
const msg = `IPFS root CID "${ipfsRootCid.toString()}" not announced to IPNI after ${maxAttempts} attempt${maxAttempts === 1 ? '' : 's'}`
110113
const error = new Error(msg)
111-
options?.logger?.error({ error }, msg)
112-
try {
113-
options?.onProgress?.({ type: 'ipniAdvertisement.complete', data: { result: false, retryCount } })
114-
} catch (error) {
115-
options?.logger?.error({ error }, 'Error in consumer onProgress callback for complete event')
116-
}
114+
options?.logger?.warn({ error }, msg)
117115
throw error
118116
}
119117
}
120118

121119
check().catch((error) => {
122-
options?.onProgress?.({ type: 'ipniAdvertisement.failed', data: { error } })
120+
try {
121+
options?.onProgress?.({ type: 'ipniAdvertisement.failed', data: { error } })
122+
} catch (callbackError) {
123+
options?.logger?.warn({ error: callbackError }, 'Error in consumer onProgress callback for failed event')
124+
}
123125
reject(error)
124126
})
125127
})

‎src/test/unit/validate-ipni-advertisement.test.ts‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ describe('validateIPNIAdvertisement', () => {
6666
})
6767

6868
describe('failed announcement', () => {
69-
it('should reject after custom maxAttempts and emit a final complete(false)', async () => {
69+
it('should reject after custom maxAttempts and emit a failed event', async () => {
7070
mockFetch.mockResolvedValue({ ok: false })
7171
const onProgress = vi.fn()
7272
const promise = validateIPNIAdvertisement(testCid, { maxAttempts: 3, onProgress })
@@ -79,13 +79,19 @@ describe('validateIPNIAdvertisement', () => {
7979
await expectPromise
8080
expect(mockFetch).toHaveBeenCalledTimes(3)
8181

82-
// Expect retryUpdate with counts 0,1,2 and final complete(false) with retryCount 3
82+
// Expect retryUpdate with counts 0,1,2 and final failed event (no complete event on failure)
8383
expect(onProgress).toHaveBeenCalledWith({ type: 'ipniAdvertisement.retryUpdate', data: { retryCount: 0 } })
8484
expect(onProgress).toHaveBeenCalledWith({ type: 'ipniAdvertisement.retryUpdate', data: { retryCount: 1 } })
8585
expect(onProgress).toHaveBeenCalledWith({ type: 'ipniAdvertisement.retryUpdate', data: { retryCount: 2 } })
86+
// Should emit failed event, not complete(false)
8687
expect(onProgress).toHaveBeenCalledWith({
88+
type: 'ipniAdvertisement.failed',
89+
data: { error: expect.any(Error) },
90+
})
91+
// Should NOT emit complete event
92+
expect(onProgress).not.toHaveBeenCalledWith({
8793
type: 'ipniAdvertisement.complete',
88-
data: { result: false, retryCount: 3 },
94+
data: { result: false, retryCount: expect.any(Number) },
8995
})
9096
})
9197

‎src/utils/cli-logger.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,9 @@ export const log = {
136136
for (const line of lines) {
137137
this.indent(line, options.indentLevel)
138138
}
139+
if (lineBuffer.length === 0) {
140+
this.newline()
141+
}
139142
this.flush()
140143
},
141144

0 commit comments

Comments
 (0)