Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/three-pens-sit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"electron-updater": minor
---

Emit download-progress events when downloading with multiple range differential downloader
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { newError } from "builder-util-runtime"
import { createReadStream } from "fs"
import { Writable } from "stream"
import { Operation, OperationKind } from "./downloadPlanBuilder"
import { ProgressInfo } from "./ProgressDifferentialDownloadCallbackTransform"

const DOUBLE_CRLF = Buffer.from("\r\n\r\n")

Expand Down Expand Up @@ -34,6 +35,11 @@ export function copyData(task: Operation, out: Writable, oldFileFd: number, reje
}

export class DataSplitter extends Writable {
private start = Date.now()
private nextUpdate = this.start + 1000
private transferred = 0
private delta = 0

partIndex = -1

private headerListBuffer: Buffer | null = null
Expand All @@ -49,7 +55,9 @@ export class DataSplitter extends Writable {
private readonly partIndexToTaskIndex: Map<number, number>,
boundary: string,
private readonly partIndexToLength: Array<number>,
private readonly finishHandler: () => any
private readonly finishHandler: () => any,
private readonly grandTotalBytes: number,
private readonly onProgress?: (info: ProgressInfo) => any
) {
super()

Expand All @@ -69,7 +77,27 @@ export class DataSplitter extends Writable {
return
}

this.handleData(data).then(callback).catch(callback)
this.handleData(data)
.then(() => {
if (this.onProgress) {
const now = Date.now()
if (now >= this.nextUpdate || this.transferred === this.grandTotalBytes) {
this.nextUpdate = now + 1000

this.onProgress({
total: this.grandTotalBytes,
delta: this.delta,
transferred: this.transferred,
percent: (this.transferred / this.grandTotalBytes) * 100,
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

Division by zero can occur if grandTotalBytes is 0. While this scenario might be unlikely, if all tasks are COPY operations or if there are no download tasks, grandTotalBytes could be 0, causing the percent calculation to result in NaN or Infinity.

Copilot uses AI. Check for mistakes.
bytesPerSecond: Math.round(this.transferred / ((now - this.start) / 1000)),
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

Division by zero can occur if the download completes in less than 1 millisecond. The calculation (now - this.start) / 1000 could result in 0 if now equals this.start, causing bytesPerSecond to be Infinity. Consider adding a guard condition or using Math.max to ensure a minimum elapsed time of 1 millisecond.

Copilot uses AI. Check for mistakes.
})
this.delta = 0
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the simplicity of this PR in that it's throttling/firing a callback when data is received (as opposed to a NodeJS.Timer approach), but I'm concerned with the number of properties that are being added.

  private start = Date.now()
  private nextUpdate = this.start + 1000
  private transferred = 0
  private delta = 0

Feels like we're recreating a wheel here but I can't think of a better approach. Can you at least please add a short tsdoc as to what each is doing (or expected to be used as/for)?


callback()
})
.catch(callback)
}

private async handleData(chunk: Buffer): Promise<undefined> {
Expand Down Expand Up @@ -217,6 +245,8 @@ export class DataSplitter extends Writable {

private processPartData(data: Buffer, start: number, end: number): Promise<void> {
this.actualPartLength += end - start
this.transferred += end - start
this.delta += end - start
const out = this.out
if (out.write(start === 0 && data.length === end ? data : data.slice(start, end))) {
return Promise.resolve()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ export abstract class DifferentialDownloader {
// Create our download info transformer if we have one
let downloadInfoTransform: ProgressDifferentialDownloadCallbackTransform | undefined = undefined
if (!this.options.isUseMultipleRangeRequest && this.options.onProgress) {
// TODO: Does not support multiple ranges (someone feel free to PR this!)
const expectedByteCounts: Array<number> = []
let grandTotalBytes = 0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export function executeTasksUsingMultipleRangeRequests(
function doExecuteTasks(differentialDownloader: DifferentialDownloader, options: PartListDataTask, out: Writable, resolve: () => void, reject: (error: Error) => void): void {
let ranges = "bytes="
let partCount = 0
let grandTotalBytes = 0
const partIndexToTaskIndex = new Map<number, number>()
const partIndexToLength: Array<number> = []
for (let i = options.start; i < options.end; i++) {
Expand All @@ -50,6 +51,7 @@ function doExecuteTasks(differentialDownloader: DifferentialDownloader, options:
partIndexToTaskIndex.set(partCount, i)
partCount++
partIndexToLength.push(task.end - task.start)
grandTotalBytes += task.end - task.start
}
}

Expand Down Expand Up @@ -103,7 +105,7 @@ function doExecuteTasks(differentialDownloader: DifferentialDownloader, options:
return
}

const dicer = new DataSplitter(out, options, partIndexToTaskIndex, m[1] || m[2], partIndexToLength, resolve)
const dicer = new DataSplitter(out, options, partIndexToTaskIndex, m[1] || m[2], partIndexToLength, resolve, grandTotalBytes, differentialDownloader.options.onProgress)
dicer.on("error", reject)
response.pipe(dicer)

Expand Down
Loading