Skip to content

Commit

Permalink
fix: preview image retry
Browse files Browse the repository at this point in the history
Adds a retry mechanism to preview loading when the server responds with `429` status codes.
  • Loading branch information
Jannik Stehle committed Oct 23, 2024
1 parent 22d7c45 commit 1b04222
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 12 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/bugfix-preview-retries
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Preview image retries

We've added a retry mechanism to preview loading in case the server is being overrun with too many requests.

https://github.com/owncloud/web/pull/11813
https://github.com/owncloud/web/issues/11798
50 changes: 38 additions & 12 deletions packages/web-pkg/src/services/preview/previewService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export class PreviewService {
}

if (isPublic) {
return this.publicPreviewUrl(options)
return this.publicPreviewUrl(options, signal)
}
try {
return await this.privatePreviewBlob(options, cached, silenceErrors, signal)
Expand All @@ -116,15 +116,19 @@ export class PreviewService {
}
}

private async cacheFactory(options: LoadPreviewOptions, silenceErrors: boolean): Promise<string> {
private async cacheFactory(
options: LoadPreviewOptions,
silenceErrors: boolean,
signal?: AbortSignal
): Promise<string> {
const { resource, dimensions } = options
const hit = cacheService.filePreview.get(resource.id.toString())

if (hit && hit.etag === resource.etag && isEqual(dimensions, hit.dimensions)) {
return hit.src
}
try {
const src = await this.privatePreviewBlob(options)
const src = await this.privatePreviewBlob(options, false, true, signal)
return cacheService.filePreview.set(
resource.id.toString(),
{ src, etag: resource.etag, dimensions },
Expand Down Expand Up @@ -158,7 +162,7 @@ export class PreviewService {
): Promise<string> {
const { resource, dimensions, processor } = options
if (cached) {
return this.cacheFactory(options, silenceErrors)
return this.cacheFactory(options, silenceErrors, signal)
}

const url = [
Expand All @@ -168,11 +172,22 @@ export class PreviewService {
'?',
this.buildQueryString({ etag: resource.etag, dimensions, processor })
].join('')
const { data } = await this.clientService.httpAuthenticated.get<Blob>(url, {
responseType: 'blob',
signal
})
return window.URL.createObjectURL(data)

try {
const { data } = await this.clientService.httpAuthenticated.get<Blob>(url, {
responseType: 'blob',
signal
})
return window.URL.createObjectURL(data)
} catch (e) {
if (e.status === 429) {
const retryAfter = e.response?.headers?.['retry-after'] || 5
await new Promise((resolve) => setTimeout(resolve, retryAfter * 1000))
return this.privatePreviewBlob(options, cached, silenceErrors, signal)
}

throw e
}
}

private async publicPreviewUrl(
Expand All @@ -194,10 +209,21 @@ export class PreviewService {
.join('&')

const previewUrl = [url, combinedQuery].filter(Boolean).join('?')
const { status } = await this.clientService.httpUnAuthenticated.head(previewUrl, { signal })

if (status !== 404) {
return previewUrl
try {
const { status } = await this.clientService.httpUnAuthenticated.head(previewUrl, { signal })

if (status !== 404) {
return previewUrl
}
} catch (e) {
if (e.status === 429) {
const retryAfter = e.response?.headers?.['retry-after'] || 5
await new Promise((resolve) => setTimeout(resolve, retryAfter * 1000))
return this.publicPreviewUrl(options, signal)
}

throw e
}
}
}
24 changes: 24 additions & 0 deletions packages/web-pkg/tests/unit/services/previewService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,30 @@ describe('PreviewService', () => {
})
expect(preview).toEqual(undefined)
})
it('retries when the server returns a 429 status code', async () => {
const supportedMimeTypes = ['image/png']
const { previewService, clientService } = getWrapper({
supportedMimeTypes,
version: '1'
})

clientService.httpAuthenticated.get.mockRejectedValueOnce({
response: { headers: { 'retry-after': 0.1 } },
status: 429
})
clientService.httpAuthenticated.get.mockResolvedValueOnce(undefined)

await previewService.loadPreview({
space: mock<SpaceResource>(),
resource: mock<Resource>({
mimeType: supportedMimeTypes[0],
webDavPath: '/',
etag: '',
canDownload: () => true
})
})
expect(clientService.httpAuthenticated.get).toHaveBeenCalledTimes(2)
})
describe('private files', () => {
it('loads preview', async () => {
const objectUrl = 'objectUrl'
Expand Down

0 comments on commit 1b04222

Please sign in to comment.