HEAD request is not retried on intermittent server errors such as 502 Bad Gateway

[nh2]

In

tus-js-client/lib/upload.js

Lines 632 to 676 in 01d3948

	* Try to resume an existing upload. First a HEAD request will be sent
	* to retrieve the offset. If the request fails a new upload will be
	* created. In the case of a successful response the file will be uploaded.
	*
	* @api private
	*/
	_resumeUpload() {
	const req = this._openRequest('HEAD', this.url)
	const promise = this._sendRequest(req, null)
	promise
	.then((res) => {
	const status = res.getStatus()
	if (!inStatusCategory(status, 200)) {
	// If the upload is locked (indicated by the 423 Locked status code), we
	// emit an error instead of directly starting a new upload. This way the
	// retry logic can catch the error and will retry the upload. An upload
	// is usually locked for a short period of time and will be available
	// afterwards.
	if (status === 423) {
	this._emitHttpError(req, res, 'tus: upload is currently locked; retry later')
	return
	}
	if (inStatusCategory(status, 400)) {
	// Remove stored fingerprint and corresponding endpoint,
	// on client errors since the file can not be found
	this._removeFromUrlStorage()
	}
	if (!this.options.endpoint) {
	// Don't attempt to create a new upload if no endpoint is provided.
	this._emitHttpError(
	req,
	res,
	'tus: unable to resume upload (new upload cannot be created without an endpoint)'
	)
	return
	}
	// Try to create a new upload
	this.url = null
	this._createUpload()
	return
	}

the logic results in HEAD requests giving e.g. HTTP 502 Bad Gateway not being retried; instead a new upload is started immediately. The user-defined onShouldRetry() is unexpectedly not called.

This is inconsistent with other parts of tus-js-client:

• PATCH requests are retried
• POST requests to create new uploads are retried on HTTP 502, because of
  

  tus-js-client/lib/upload.js

  Lines 592 to 593 in 01d3948

  	if (!inStatusCategory(res.getStatus(), 200)) {
  	this._emitHttpError(req, res, 'tus: unexpected response while creating upload')

  
  where _emitHttpError() calls _emitError() which has the retry logic (shouldRetry() calling onShouldRetry())

To Reproduce

1. Start an upload, e.g. to some TUS server with a reverse proxy such as nginx in between
2. Restart the upstream TUS server, so that HTTP 502 Bad Gateway is generated by the reverse proxy
3. See error

Please try to reproduce the problem when uploading to our public tus instance at https://tusd.tusdemo.net/files/, if applicable to your situation. This helps us to determine whether the problem may be caused by the server component.

Expected behavior

• HEAD retries the same way as PATCH.
• onShouldRetry() should be called for HEAD the same way as for PATCH, so that the user can implement error handling.

[nh2]

It also looks to me that

tus-js-client/lib/upload.js

Lines 656 to 660 in 01d3948

	if (inStatusCategory(status, 400)) {
	// Remove stored fingerprint and corresponding endpoint,
	// on client errors since the file can not be found
	this._removeFromUrlStorage()
	}

is too general.

For example, HTTP 429 Too Many Requests for rate-limiting should allow the user to back off and retry after a short time, instead of creating a new upoad (thus creating even more requests).

It seems that a better approach is to list exactly the cases in which new uploads are created without calling the user's onShouldRetry(), instead of this broad handling.

Similar to

tus-js-client/lib/upload.js

Line 1013 in 01d3948

return (!inStatusCategory(status, 400) || status === 409 || status === 423) && isOnline()

where cases are also listed explicitly.

[Acconut]

For example, HTTP 429 Too Many Requests for rate-limiting should allow the user to back off and retry after a short time, instead of creating a new upoad (thus creating even more requests).

Good point. We can add an exception for 429 to handle it the same way as 423 is done.