diff --git a/lib/DBSQLSession.ts b/lib/DBSQLSession.ts index a9bbbb2e..242d6d34 100644 --- a/lib/DBSQLSession.ts +++ b/lib/DBSQLSession.ts @@ -287,11 +287,14 @@ export default class DBSQLSession implements IDBSQLSession { const agent = await connectionProvider.getAgent(); const response = await fetch(presignedUrl, { method: 'DELETE', headers, agent }); - // Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files - // AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200 + // Looks that AWS and Azure have a different behavior of HTTP `DELETE` for non-existing files. + // AWS assumes that - since file already doesn't exist - the goal is achieved, and returns HTTP 200. // Azure, on the other hand, is somewhat stricter and check if file exists before deleting it. And if - // file doesn't exist - Azure returns HTTP 404 - if (!response.ok) { + // file doesn't exist - Azure returns HTTP 404. + // + // For us, it's totally okay if file didn't exist before removing. So when we get an HTTP 404 - + // just ignore it and report success. This way we can have a uniform library behavior for all clouds + if (!response.ok && response.status !== 404) { throw new StagingError(`HTTP error ${response.status} ${response.statusText}`); } } diff --git a/tests/e2e/staging_ingestion.test.js b/tests/e2e/staging_ingestion.test.js index d8feb423..b9cd3fea 100644 --- a/tests/e2e/staging_ingestion.test.js +++ b/tests/e2e/staging_ingestion.test.js @@ -114,17 +114,25 @@ describe('Staging Test', () => { }); const stagingFileName = `/Volumes/${catalog}/${schema}/${volume}/${uuid.v4()}.csv`; + const localFile = path.join(localPath, `${uuid.v4()}.csv`); + // File should not exist before removing try { - await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] }); - // In some cases, `REMOVE` may silently succeed for non-existing files (see comment in relevant - // part of `DBSQLSession` code). But if it fails - it has to be an HTTP 404 error + await session.executeStatement(`GET '${stagingFileName}' TO '${localFile}'`, { + stagingAllowedLocalPath: [localPath], + }); + expect.fail('It should throw HTTP 404 error'); } catch (error) { if (error instanceof StagingError) { expect(error.message).to.contain('404'); } else { throw error; } + } finally { + fs.rmSync(localFile, { force: true }); } + + // Try to remove the file - it should succeed and not throw any errors + await session.executeStatement(`REMOVE '${stagingFileName}'`, { stagingAllowedLocalPath: [localPath] }); }); });