From 18ebc61b27263437f6b3814fb774e65c2c4f3b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jind=C5=99ich=20B=C3=A4r?= Date: Wed, 20 May 2026 16:12:04 +0200 Subject: [PATCH] fix(core): release storage open queue slot on failure (#3662) A failing `StorageManager.openStorage()` call (e.g. invalid storage client token) left its slot in `storageOpenQueue` held forever, deadlocking every subsequent `openStorage` call on the same manager. The body now runs inside a `try/finally` so the slot is always released. Includes a regression test that times out against the unpatched code. Closes #3661 --- packages/core/src/storages/storage_manager.ts | 50 ++++++++++--------- test/core/storages/storage_manager.test.ts | 37 ++++++++++++++ 2 files changed, 63 insertions(+), 24 deletions(-) create mode 100644 test/core/storages/storage_manager.test.ts diff --git a/packages/core/src/storages/storage_manager.ts b/packages/core/src/storages/storage_manager.ts index aea40468c9ca..6e5426c3b4f4 100644 --- a/packages/core/src/storages/storage_manager.ts +++ b/packages/core/src/storages/storage_manager.ts @@ -70,32 +70,34 @@ export class StorageManager { async openStorage(idOrName?: string | null, client?: StorageClient): Promise { await this.storageOpenQueue.wait(); - if (!idOrName) { - const defaultIdConfigKey = DEFAULT_ID_CONFIG_KEYS[this.name]; - idOrName = this.config.get(defaultIdConfigKey) as string; - } - - const cacheKey = idOrName; - let storage = this.cache.get(cacheKey); - - if (!storage) { - client ??= this.config.getStorageClient(); - const storageObject = await this._getOrCreateStorage(idOrName, this.name, client); - storage = new this.StorageConstructor( - { - id: storageObject.id, - name: storageObject.name, - storageObject, - client, - }, - this.config, - ); - this._addStorageToCache(storage); - } + try { + if (!idOrName) { + const defaultIdConfigKey = DEFAULT_ID_CONFIG_KEYS[this.name]; + idOrName = this.config.get(defaultIdConfigKey) as string; + } - this.storageOpenQueue.shift(); + const cacheKey = idOrName; + let storage = this.cache.get(cacheKey); + + if (!storage) { + client ??= this.config.getStorageClient(); + const storageObject = await this._getOrCreateStorage(idOrName, this.name, client); + storage = new this.StorageConstructor( + { + id: storageObject.id, + name: storageObject.name, + storageObject, + client, + }, + this.config, + ); + this._addStorageToCache(storage); + } - return storage; + return storage; + } finally { + this.storageOpenQueue.shift(); + } } closeStorage(storage: { id: string; name?: string }): void { diff --git a/test/core/storages/storage_manager.test.ts b/test/core/storages/storage_manager.test.ts new file mode 100644 index 000000000000..2682ba61d68a --- /dev/null +++ b/test/core/storages/storage_manager.test.ts @@ -0,0 +1,37 @@ +import { Configuration, Dataset } from '@crawlee/core'; + +import { MemoryStorageEmulator } from '../../shared/MemoryStorageEmulator'; + +const localStorageEmulator = new MemoryStorageEmulator(); + +beforeEach(async () => { + await localStorageEmulator.init(); +}); + +afterAll(async () => { + await localStorageEmulator.destroy(); +}); + +describe('StorageManager', () => { + test('failed openStorage call does not block subsequent calls (#3661)', async () => { + const goodClient = Configuration.getStorageClient(); + const failingClient = { + ...goodClient, + datasets: () => { + throw new Error('boom'); + }, + dataset: () => { + throw new Error('boom'); + }, + }; + + await expect(Dataset.open('will-fail', { storageClient: failingClient as any })).rejects.toThrow('boom'); + + await expect( + Promise.race([ + Dataset.open('fallback'), + new Promise((_, reject) => setTimeout(() => reject(new Error('timeout')), 1000)), + ]), + ).resolves.toBeDefined(); + }); +});