diff --git a/src/storage/object/get.js b/src/storage/object/get.js index 140f7945..f7294368 100644 --- a/src/storage/object/get.js +++ b/src/storage/object/get.js @@ -74,12 +74,12 @@ export default async function getObject( etag: resp.ETag, }; } catch (e) { - if (!e.$metadata?.httpStatusCode) { - // eslint-disable-next-line no-console - console.error('Error getting object without httpStatusCode', e); - } // Handle conditional request failures (304 Not Modified, 412 Precondition Failed) const status = e.$metadata?.httpStatusCode || 500; + // eslint-disable-next-line no-console + if (status >= 500) { + console.error(e.$metadata?.httpStatusCode ? 'Error getting object' : 'Error getting object without httpStatusCode', e); + } if (status === 304 || status === 412) { // Include ETag in 304/412 responses per RFC 7232 return { diff --git a/test/storage/object/get.test.js b/test/storage/object/get.test.js index ec18a0b9..92e4e0e6 100644 --- a/test/storage/object/get.test.js +++ b/test/storage/object/get.test.js @@ -128,4 +128,62 @@ describe('Get Object', () => { assert.strictEqual(resp.body, ''); assert.strictEqual(resp.contentLength, 0); }); + + it('logs the error when S3/R2 returns a 500 (get_failed path produces empty Cloudflare Logs)', async () => { + // Regression: a backend 500 that carries $metadata.httpStatusCode was never logged, + // so Cloudflare Logs was empty and the root cause was invisible in production. + const error = new Error('We encountered an internal error. Please try again.'); + error.$metadata = { httpStatusCode: 500 }; + s3Mock.on(GetObjectCommand, { + Bucket: BUCKET, + Key: S3_KEY, + }).rejects(error); + + const errors = []; + const origError = console.error; + console.error = (...args) => { + errors.push(args); + }; + let resp; + try { + // eslint-disable-next-line no-shadow + const getObject = (await import('../../../src/storage/object/get.js')).default; + resp = await getObject({}, { bucket: BUCKET, org: ORG, key: KEY }, false); + } finally { + console.error = origError; + } + + assert.strictEqual(resp.status, 500); + assert.strictEqual(resp.error, error.message); + assert(errors.length > 0, 'get_failed must log the error so it appears in Cloudflare Logs'); + assert.strictEqual(errors[0][0], 'Error getting object'); + assert.strictEqual(errors[0][1], error); + }); + + it('logs a distinct message when the error has no httpStatusCode (network/timeout failure)', async () => { + const error = new Error('Network connection lost'); + s3Mock.on(GetObjectCommand, { + Bucket: BUCKET, + Key: S3_KEY, + }).rejects(error); + + const errors = []; + const origError = console.error; + console.error = (...args) => { + errors.push(args); + }; + let resp; + try { + // eslint-disable-next-line no-shadow + const getObject = (await import('../../../src/storage/object/get.js')).default; + resp = await getObject({}, { bucket: BUCKET, org: ORG, key: KEY }, false); + } finally { + console.error = origError; + } + + assert.strictEqual(resp.status, 500); + assert(errors.length > 0, 'must log the error so it appears in Cloudflare Logs'); + assert.strictEqual(errors[0][0], 'Error getting object without httpStatusCode'); + assert.strictEqual(errors[0][1], error); + }); });