diff --git a/docs/src/api/class-download.md b/docs/src/api/class-download.md index 2ae2da377c..8f54048c9f 100644 --- a/docs/src/api/class-download.md +++ b/docs/src/api/class-download.md @@ -70,7 +70,7 @@ Upon successful cancellations, `download.failure()` would resolve to `'canceled' ## async method: Download.createReadStream * since: v1.8 * langs: java, js, csharp -- returns: <[null]|[Readable]> +- returns: <[Readable]> Returns readable stream for current download or `null` if download failed. @@ -93,7 +93,7 @@ Get the page that the download belongs to. ## async method: Download.path * since: v1.8 -- returns: <[null]|[path]> +- returns: <[path]> Returns path to the downloaded file in case of successful download. The method will wait for the download to finish if necessary. The method throws when connected remotely. diff --git a/packages/playwright-core/src/client/artifact.ts b/packages/playwright-core/src/client/artifact.ts index 84fc9ef983..f93dd98541 100644 --- a/packages/playwright-core/src/client/artifact.ts +++ b/packages/playwright-core/src/client/artifact.ts @@ -26,10 +26,10 @@ export class Artifact extends ChannelOwner { return (channel as any)._object; } - async pathAfterFinished(): Promise { + async pathAfterFinished(): Promise { if (this._connection.isRemote()) throw new Error(`Path is not available when connecting remotely. Use saveAs() to save a local copy.`); - return (await this._channel.pathAfterFinished()).value || null; + return (await this._channel.pathAfterFinished()).value; } async saveAs(path: string): Promise { @@ -52,10 +52,8 @@ export class Artifact extends ChannelOwner { return (await this._channel.failure()).error || null; } - async createReadStream(): Promise { + async createReadStream(): Promise { const result = await this._channel.stream(); - if (!result.stream) - return null; const stream = Stream.from(result.stream); return stream.stream(); } diff --git a/packages/playwright-core/src/client/download.ts b/packages/playwright-core/src/client/download.ts index 257419a0e6..22b08be657 100644 --- a/packages/playwright-core/src/client/download.ts +++ b/packages/playwright-core/src/client/download.ts @@ -44,7 +44,7 @@ export class Download implements api.Download { return this._suggestedFilename; } - async path(): Promise { + async path(): Promise { return this._artifact.pathAfterFinished(); } @@ -56,7 +56,7 @@ export class Download implements api.Download { return this._artifact.failure(); } - async createReadStream(): Promise { + async createReadStream(): Promise { return this._artifact.createReadStream(); } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 706512bb9f..229f4683c6 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -2161,7 +2161,7 @@ scheme.ArtifactInitializer = tObject({ }); scheme.ArtifactPathAfterFinishedParams = tOptional(tObject({})); scheme.ArtifactPathAfterFinishedResult = tObject({ - value: tOptional(tString), + value: tString, }); scheme.ArtifactSaveAsParams = tObject({ path: tString, @@ -2177,7 +2177,7 @@ scheme.ArtifactFailureResult = tObject({ }); scheme.ArtifactStreamParams = tOptional(tObject({})); scheme.ArtifactStreamResult = tObject({ - stream: tOptional(tChannel(['Stream'])), + stream: tChannel(['Stream']), }); scheme.ArtifactCancelParams = tOptional(tObject({})); scheme.ArtifactCancelResult = tOptional(tObject({})); diff --git a/packages/playwright-core/src/server/artifact.ts b/packages/playwright-core/src/server/artifact.ts index 9d1b239e0b..f82d092211 100644 --- a/packages/playwright-core/src/server/artifact.ts +++ b/packages/playwright-core/src/server/artifact.ts @@ -18,8 +18,9 @@ import fs from 'fs'; import { assert } from '../utils'; import { ManualPromise } from '../utils/manualPromise'; import { SdkObject } from './instrumentation'; +import { TargetClosedError } from '../common/errors'; -type SaveCallback = (localPath: string, error?: string) => Promise; +type SaveCallback = (localPath: string, error?: Error) => Promise; type CancelCallback = () => Promise; export class Artifact extends SdkObject { @@ -30,7 +31,7 @@ export class Artifact extends SdkObject { private _saveCallbacks: SaveCallback[] = []; private _finished: boolean = false; private _deleted = false; - private _failureError: string | null = null; + private _failureError: Error | undefined; constructor(parent: SdkObject, localPath: string, unaccessibleErrorMessage?: string, cancelCallback?: CancelCallback) { super(parent, 'artifact'); @@ -47,12 +48,12 @@ export class Artifact extends SdkObject { return this._localPath; } - async localPathAfterFinished(): Promise { + async localPathAfterFinished(): Promise { if (this._unaccessibleErrorMessage) throw new Error(this._unaccessibleErrorMessage); await this._finishedPromise; if (this._failureError) - return null; + throw this._failureError; return this._localPath; } @@ -62,10 +63,10 @@ export class Artifact extends SdkObject { if (this._deleted) throw new Error(`File already deleted. Save before deleting.`); if (this._failureError) - throw new Error(`File not found on disk. Check download.failure() for details.`); + throw this._failureError; if (this._finished) { - saveCallback(this._localPath).catch(e => {}); + saveCallback(this._localPath).catch(() => {}); return; } this._saveCallbacks.push(saveCallback); @@ -75,7 +76,7 @@ export class Artifact extends SdkObject { if (this._unaccessibleErrorMessage) return this._unaccessibleErrorMessage; await this._finishedPromise; - return this._failureError; + return this._failureError?.message || null; } async cancel(): Promise { @@ -102,14 +103,14 @@ export class Artifact extends SdkObject { this._deleted = true; if (!this._unaccessibleErrorMessage) await fs.promises.unlink(this._localPath).catch(e => {}); - await this.reportFinished('File deleted upon browser context closure.'); + await this.reportFinished(new TargetClosedError()); } - async reportFinished(error?: string) { + async reportFinished(error?: Error) { if (this._finished) return; this._finished = true; - this._failureError = error || null; + this._failureError = error; if (error) { for (const callback of this._saveCallbacks) diff --git a/packages/playwright-core/src/server/browser.ts b/packages/playwright-core/src/server/browser.ts index 711d68b06d..0d740b1bf8 100644 --- a/packages/playwright-core/src/server/browser.ts +++ b/packages/playwright-core/src/server/browser.ts @@ -119,7 +119,7 @@ export abstract class Browser extends SdkObject { const download = this._downloads.get(uuid); if (!download) return; - download.artifact.reportFinished(error); + download.artifact.reportFinished(error ? new Error(error) : undefined); this._downloads.delete(uuid); } diff --git a/packages/playwright-core/src/server/chromium/crBrowser.ts b/packages/playwright-core/src/server/chromium/crBrowser.ts index e7e66ebe02..498f08298d 100644 --- a/packages/playwright-core/src/server/chromium/crBrowser.ts +++ b/packages/playwright-core/src/server/chromium/crBrowser.ts @@ -269,7 +269,7 @@ export class CRBrowser extends Browser { if (payload.state === 'completed') this._downloadFinished(payload.guid, ''); if (payload.state === 'canceled') - this._downloadFinished(payload.guid, 'canceled'); + this._downloadFinished(payload.guid, this._closeReason || 'canceled'); } async _closePage(crPage: CRPage) { diff --git a/packages/playwright-core/src/server/dispatchers/artifactDispatcher.ts b/packages/playwright-core/src/server/dispatchers/artifactDispatcher.ts index d898e19e61..11e161fe49 100644 --- a/packages/playwright-core/src/server/dispatchers/artifactDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/artifactDispatcher.ts @@ -44,14 +44,14 @@ export class ArtifactDispatcher extends Dispatcher { const path = await this._object.localPathAfterFinished(); - return { value: path || undefined }; + return { value: path }; } async saveAs(params: channels.ArtifactSaveAsParams): Promise { return await new Promise((resolve, reject) => { this._object.saveAs(async (localPath, error) => { - if (error !== undefined) { - reject(new Error(error)); + if (error) { + reject(error); return; } try { @@ -68,8 +68,8 @@ export class ArtifactDispatcher extends Dispatcher { return await new Promise((resolve, reject) => { this._object.saveAs(async (localPath, error) => { - if (error !== undefined) { - reject(new Error(error)); + if (error) { + reject(error); return; } try { @@ -92,8 +92,6 @@ export class ArtifactDispatcher extends Dispatcher { const fileName = await this._object.localPathAfterFinished(); - if (!fileName) - return {}; const readable = fs.createReadStream(fileName, { highWaterMark: 1024 * 1024 }); return { stream: new StreamDispatcher(this, readable) }; } diff --git a/packages/playwright-core/src/server/firefox/ffBrowser.ts b/packages/playwright-core/src/server/firefox/ffBrowser.ts index d299e4c734..38233ea3dc 100644 --- a/packages/playwright-core/src/server/firefox/ffBrowser.ts +++ b/packages/playwright-core/src/server/firefox/ffBrowser.ts @@ -15,7 +15,7 @@ * limitations under the License. */ -import { kTargetClosedErrorMessage } from '../../common/errors'; +import { TargetClosedError } from '../../common/errors'; import { assert } from '../../utils'; import type { BrowserOptions } from '../browser'; import { Browser } from '../browser'; @@ -159,7 +159,7 @@ export class FFBrowser extends Browser { _onDisconnect() { for (const video of this._idToVideo.values()) - video.artifact.reportFinished(kTargetClosedErrorMessage); + video.artifact.reportFinished(new TargetClosedError()); this._idToVideo.clear(); for (const ffPage of this._ffPages.values()) ffPage.didClose(); diff --git a/packages/playwright-core/src/server/webkit/wkBrowser.ts b/packages/playwright-core/src/server/webkit/wkBrowser.ts index fbf0cda3a2..db452a21dc 100644 --- a/packages/playwright-core/src/server/webkit/wkBrowser.ts +++ b/packages/playwright-core/src/server/webkit/wkBrowser.ts @@ -30,7 +30,7 @@ import type { Protocol } from './protocol'; import type { PageProxyMessageReceivedPayload } from './wkConnection'; import { kPageProxyMessageReceived, WKConnection, WKSession } from './wkConnection'; import { WKPage } from './wkPage'; -import { kTargetClosedErrorMessage } from '../../common/errors'; +import { TargetClosedError } from '../../common/errors'; import type { SdkObject } from '../instrumentation'; const DEFAULT_USER_AGENT = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.4 Safari/605.1.15'; @@ -81,7 +81,7 @@ export class WKBrowser extends Browser { wkPage.didClose(); this._wkPages.clear(); for (const video of this._idToVideo.values()) - video.artifact.reportFinished(kTargetClosedErrorMessage); + video.artifact.reportFinished(new TargetClosedError()); this._idToVideo.clear(); this._didClose(); } diff --git a/packages/playwright-core/types/types.d.ts b/packages/playwright-core/types/types.d.ts index bec55023c5..91552b7eb3 100644 --- a/packages/playwright-core/types/types.d.ts +++ b/packages/playwright-core/types/types.d.ts @@ -16897,7 +16897,7 @@ export interface Download { /** * Returns readable stream for current download or `null` if download failed. */ - createReadStream(): Promise; + createReadStream(): Promise; /** * Deletes the downloaded file. Will wait for the download to finish if necessary. @@ -16922,7 +16922,7 @@ export interface Download { * [download.suggestedFilename()](https://playwright.dev/docs/api/class-download#download-suggested-filename) to get * suggested file name. */ - path(): Promise; + path(): Promise; /** * Copy the download to a user-specified path. It is safe to call this method while the download is still in progress. diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts index a20fbaeca6..59adbebf61 100644 --- a/packages/protocol/src/channels.ts +++ b/packages/protocol/src/channels.ts @@ -3862,7 +3862,7 @@ export interface ArtifactChannel extends ArtifactEventTarget, Channel { export type ArtifactPathAfterFinishedParams = {}; export type ArtifactPathAfterFinishedOptions = {}; export type ArtifactPathAfterFinishedResult = { - value?: string, + value: string, }; export type ArtifactSaveAsParams = { path: string, @@ -3884,7 +3884,7 @@ export type ArtifactFailureResult = { export type ArtifactStreamParams = {}; export type ArtifactStreamOptions = {}; export type ArtifactStreamResult = { - stream?: StreamChannel, + stream: StreamChannel, }; export type ArtifactCancelParams = {}; export type ArtifactCancelOptions = {}; diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml index 3bc8fa35ab..bfff9ff756 100644 --- a/packages/protocol/src/protocol.yml +++ b/packages/protocol/src/protocol.yml @@ -3056,7 +3056,7 @@ Artifact: pathAfterFinished: returns: - value: string? + value: string # Blocks path/failure/delete/context.close until saved to the local |path|. saveAs: @@ -3074,7 +3074,7 @@ Artifact: stream: returns: - stream: Stream? + stream: Stream cancel: diff --git a/tests/library/download.spec.ts b/tests/library/download.spec.ts index 893631c263..9fd0656f33 100644 --- a/tests/library/download.spec.ts +++ b/tests/library/download.spec.ts @@ -19,6 +19,7 @@ import fs from 'fs'; import path from 'path'; import crypto from 'crypto'; import type { Download } from 'playwright-core'; +import { kTargetClosedErrorMessage } from '../config/errors'; it.describe('download event', () => { it.skip(({ mode }) => mode !== 'default', 'download.path() is not available in remote mode'); @@ -115,7 +116,7 @@ it.describe('download event', () => { expect(download.suggestedFilename()).toBe(`file.txt`); await download.path().catch(e => error = e); expect(await download.failure()).toContain('acceptDownloads'); - expect(error.message).toContain('acceptDownloads: true'); + expect(error!.message).toContain('acceptDownloads: true'); await page.close(); }); @@ -421,12 +422,12 @@ it.describe('download event', () => { // probably because of http -> https link. page.click('a', { modifiers: ['Alt'] }) ]); - const [downloadPath, saveError] = await Promise.all([ - download.path(), + const [downloadError, saveError] = await Promise.all([ + download.path().catch(e => e), download.saveAs(testInfo.outputPath('download.txt')).catch(e => e), page.context().close(), ]); - expect(downloadPath).toBe(null); + expect(downloadError.message).toBe('download.path: canceled'); expect([ 'download.saveAs: File not found on disk. Check download.failure() for details.', 'download.saveAs: canceled', @@ -451,17 +452,20 @@ it.describe('download event', () => { page.waitForEvent('download'), page.click('a') ]); - const [downloadPath, saveError] = await Promise.all([ - download.path(), + const [downloadError, saveError] = await Promise.all([ + download.path().catch(e => e), download.saveAs(testInfo.outputPath('download.txt')).catch(e => e), page.context().close(), ]); - expect(downloadPath).toBe(null); // The exact error message is racy, because sometimes browser is fast enough // to cancel the download. + expect([ + 'download.path: canceled', + 'download.path: ' + kTargetClosedErrorMessage, + ]).toContain(downloadError.message); expect([ 'download.saveAs: canceled', - 'download.saveAs: File deleted upon browser context closure.', + 'download.saveAs: ' + kTargetClosedErrorMessage, ]).toContain(saveError.message); }); @@ -482,13 +486,13 @@ it.describe('download event', () => { page.waitForEvent('download'), page.click('a') ]); - const [downloadPath, saveError] = await Promise.all([ - download.path(), + const [downloadError, saveError] = await Promise.all([ + download.path().catch(e => e), download.saveAs(testInfo.outputPath('download.txt')).catch(e => e), (browser as any)._channel.killForTests(), ]); - expect(downloadPath).toBe(null); - expect(saveError.message).toContain('File deleted upon browser context closure.'); + expect(downloadError.message).toBe('download.path: ' + kTargetClosedErrorMessage); + expect(saveError.message).toContain('download.saveAs: ' + kTargetClosedErrorMessage); await browser.close(); }); @@ -512,10 +516,10 @@ it.describe('download event', () => { const stream = await download.createReadStream(); const data = await new Promise((fulfill, reject) => { - const bufs = []; - stream.on('data', d => bufs.push(d)); + const buffs: Buffer[] = []; + stream.on('data', d => buffs.push(d)); stream.on('error', reject); - stream.on('end', () => fulfill(Buffer.concat(bufs))); + stream.on('end', () => fulfill(Buffer.concat(buffs))); }); expect(data.byteLength).toBe(content.byteLength); expect(data.equals(content)).toBe(true); @@ -585,7 +589,7 @@ it.describe('download event', () => { page.waitForEvent('download'), page.frame({ url: server.PREFIX + '/3' - }).click('text=download') + })!.click('text=download') ]); const userPath = testInfo.outputPath('download.txt'); await download.saveAs(userPath); @@ -733,10 +737,10 @@ async function assertDownloadToPDF(download: Download, filePath: string) { expect(download.suggestedFilename()).toBe(path.basename(filePath)); const stream = await download.createReadStream(); const data = await new Promise((fulfill, reject) => { - const bufs = []; - stream.on('data', d => bufs.push(d)); + const buffs: Buffer[] = []; + stream.on('data', d => buffs.push(d)); stream.on('error', reject); - stream.on('end', () => fulfill(Buffer.concat(bufs))); + stream.on('end', () => fulfill(Buffer.concat(buffs))); }); expect(download.url().endsWith('/' + path.basename(filePath))).toBeTruthy(); const expectedPrefix = '%PDF';