Handling of 429 errors is weird

[benoit74]

mwoffliner/src/Downloader.ts

Lines 457 to 478 in 7c3857a

	private getJSONCb = <T>(url: string, handler: (...args: any[]) => any): void => {
	logger.info(`Getting JSON from [${url}]`)
	axios
	.get<T>(url, this.jsonRequestOptions)
	.then((a) => handler(null, a.data), handler)
	.catch((err) => {
	try {
	if (err.response && err.response.status === 429) {
	logger.log('Received a [status=429], slowing down')
	const newMaxActiveRequests: number = Math.max(this.maxActiveRequests - 1, 1)
	logger.log(`Setting maxActiveRequests from [${this.maxActiveRequests}] to [${newMaxActiveRequests}]`)
	this.maxActiveRequests = newMaxActiveRequests
	return this.getJSONCb(url, handler)
	} else if (err.response && err.response.status === 404) {
	handler(err)
	}
	} catch (a) {
	logger.log('ERR', err)
	handler(err)
	}
	})
	}

I have following interrogations:

• why do we only decrease the level of parallelism? to me, the general intent of 429 error is to ask for a pause, not to ask for less requests in parallel (even if obviously this has an impact)
• this code looks like a trap, there is no escape, we might fell stuck in this loop forever since we might continue to call too often, and the upstream might continue to ask to slow down with a 429 error
• why don't we use the backoff mechanism which is already in place, so that we make a pause + the pause is exponentially long?
• why is this implement only for getJSONCb method? 429 could happen in any HTTP call (aside S3 ones probably) so this should be more generic from my PoV

WDYT?