-
-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Forward 304 and If-Modified-Since/If-None-Match headers #66
Conversation
@@ -84,6 +86,9 @@ async function requestHandler( | |||
} | |||
return response.send(data.body) | |||
} catch (err) { | |||
if (err.$metadata?.httpStatusCode === 304) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does AWS sdk v2 also behave like this? Any chance of fixing it upstream? If not, add a comment here that aws sdk throws for 304
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a comment in aws/aws-sdk-js-v3#2635 questioning whether 304 should be thrown as an exception. However, I tested AWS SDK v2:
const AWS = require('aws-sdk')
const s3 = new AWS.S3()
s3.getObject({Bucket: '...', Key: '...', IfNoneMatch: '...'}, (err) => console.log(err))
And got error:
NotModified: null
at Request.extractError (/Users/beng/supabase/node_modules/aws-sdk/lib/services/s3.js:694:35)
at Request.callListeners (/Users/beng/supabase/node_modules/aws-sdk/lib/sequential_executor.js:106:20)
at Request.emit (/Users/beng/supabase/node_modules/aws-sdk/lib/sequential_executor.js:78:10)
at Request.emit (/Users/beng/supabase/node_modules/aws-sdk/lib/request.js:688:14)
at Request.transition (/Users/beng/supabase/node_modules/aws-sdk/lib/request.js:22:10)
at AcceptorStateMachine.runTo (/Users/beng/supabase/node_modules/aws-sdk/lib/state_machine.js:14:12)
at /Users/beng/supabase/node_modules/aws-sdk/lib/state_machine.js:26:10
at Request.<anonymous> (/Users/beng/supabase/node_modules/aws-sdk/lib/request.js:38:9)
at Request.<anonymous> (/Users/beng/supabase/node_modules/aws-sdk/lib/request.js:690:12)
at Request.callListeners (/Users/beng/supabase/node_modules/aws-sdk/lib/sequential_executor.js:116:18) {
code: 'NotModified',
region: null,
time: 2021-08-17T06:13:24.119Z,
requestId: 'GZXPQZGF4FKQC3XY',
extendedRequestId: '7fXRjHwQPkd8rD80mFhRj6s/y/gxhTp7Z4fsK6fI34TdvXl3J4giEEZgkAczYZzUm9WTGL21Mr0=',
cfId: undefined,
statusCode: 304,
retryable: false,
retryDelay: 6.728630404646108
}
Since this behaviour is consistent between AWS SDK v2 and v3, I don't think it'll be fixed upstream, and don't think it warrants a comment since it's evident that AWS SDK throws for 304?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh its the same as v2..yeah then I dont think this will be fixed..
Amazing Beng! |
🎉 This PR is included in version 0.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Resolves #60.