-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
domainName should be checked before replacing #1382
Conversation
@@ -2,5 +2,5 @@ export const s3BucketNameFromEventRequest = ( | |||
request: AWSLambda.CloudFrontRequest | |||
): string | undefined => { | |||
const { region, domainName } = request.origin?.s3 || {}; | |||
return domainName?.replace(`.s3.${region}.amazonaws.com`, ""); | |||
return domainName?.includes(region) ? domainName.replace(`.s3.${region}.amazonaws.com`, "") : domainName.replace(`.s3.amazonaws.com`, ""); |
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 believe this is the automated Codacy suggestion (though it doesn't seem correct?), though the code build itself was also failing due to type errors, so that might be why it made that suggestion.
Looks like some unit tests have failed. E.g previously was expecting:
but now bucket is |
Now 'Bucket: "my-bucket"' is expected.
Handler Size Report
Base Handler Sizes (kB) (commit 28c8bd6){
"Lambda@Edge": {
"Default Lambda": {
"Standard": 1231,
"Minified": 435
},
"API Lambda": {
"Standard": 85,
"Minified": 34
},
"Image Lambda": {
"Standard": 903,
"Minified": 357
},
"Regeneration Lambda": {
"Standard": 629,
"Minified": 223
}
}
} New Handler Sizes (kB) (commit 3be15e4){
"Lambda@Edge": {
"Default Lambda": {
"Standard": 1226,
"Minified": 432
},
"API Lambda": {
"Standard": 86,
"Minified": 34
},
"Image Lambda": {
"Standard": 894,
"Minified": 354
},
"Regeneration Lambda": {
"Standard": 620,
"Minified": 220
}
}
} |
Codecov Report
@@ Coverage Diff @@
## master #1382 +/- ##
==========================================
+ Coverage 83.86% 84.14% +0.27%
==========================================
Files 97 96 -1
Lines 3371 3367 -4
Branches 1001 995 -6
==========================================
+ Hits 2827 2833 +6
+ Misses 482 474 -8
+ Partials 62 60 -2
Continue to review full report at Codecov.
|
Manually ran e2e tests here: https://github.com/serverless-nextjs/serverless-next.js/actions/runs/1013949580. Will merge after it passes. |
Issue has been described here: #1381