-
Notifications
You must be signed in to change notification settings - Fork 38
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
Requests to S3 are malformed / missing bucket name in WebServerHandler Lambda #115
Comments
This is the setup I used in jetbridge/cdk-nextjs#115 **however note that I removed my account number from the CDK stack** - it should be clear where a person would enter their account number or region.
Just incase this is useful to anybody, https://github.com/kevin-mitchell/nextjs-cdk-test is exactly what I deployed (though you may need to enter your account number you're deploying to. I actually can probably remove that property but for now it's there due to copy / pasta). |
I was alerted by somebody in a Discord conversation (thanks Jack M!) that this issue popped up for him with the 3.x release of
to
I was able to deploy the Next.js sample app and everything worked... so what changed? I guess that's the next thing to look at. |
OK I'm not sure if this is useful or not (it might very well be I'm pulling on the wrong thread here), but
open-next then builds a bunch of stuff (where I need to go deeper next), but ultimately the place that the bucket name is used is here: https://github.com/serverless-stack/open-next/blob/45d67314748c0875ad9972203272a3e1328af808/packages/open-next/src/adapters/cache.ts#L81 From what I can tell, that env variable is set outside of |
@kevin-mitchell I've made progress! so the issue stems from the
In |
I think we should update CDK-nextjs for the new open-next, not sure what's involved. maybe @khuezy knows PRs always welcome! |
@jlegreid this is awesome, thank you! I'm new to all of this, but it certainly seems like the correct course should be to fix things so that the latest features of open-next and the latest stable release is supported. I'm actually out away from a computer the next week but will try to keep looking into this when I get back in town. Obviously others might get to it first, which is also great of course :) Thank you!! |
@jlegreid did you ever make any progress on this? I stepped away (vacation, work work, etc) for a while and am just getting back to this. I'm new to this entire stack (Next.js and SST most significantly, although I'm not a CDK expert either) so it's a sort of a lot to take in all at once, but putting the pieces together it seems like this issue is very much related to #119 - it's the update in open-next to support ISR that is causing this issue. Your fix, which makes sense because it's pre-open-next-v2, doesn't seem to work currently because of this issue: sst/open-next#159 Honestly I don't understand how the dependencies play together here because it seems like by specifying an older version of open-next we wouldn't have this issue, but when I attempt to build with
It really seems like it would be ideal to update this package to work with the current version of open-next, i.e. to support ISR and such. But for now, just fixing the above issue would be nice. |
@kevin-mitchell To be honest I haven't spent any more time on the issue. After specifying the open-next version in my cdk-stack I haven't had any other issues. Unfortunately with our limited resources on our team we probably won't be able to spend more time digging into it as long as this keeps working for us. |
This was resolved in #119. Please re-open if it's still an issue! |
Taken at face value, it looks like this parameter is missing from a configuration value somewhere in the Next.js app, deployed Lambda, etc. I'm not sure where this comes from yet, or who / what is responsible for supplying it, but it seems like it should be required. Also, it seems at least possible this is just a red herring for something bigger / different, but I'm creating this issue as a starting point.
Here are the steps I took to reproduce this error:
Create new AWS Account
To do this I just created a brand new account in my organization.
Start a brand new NextJS project
npx create-next-app@latest
Note: here are the resulting dep versions:
npx cdk init sample-app —language typescript
Note: here are the resulting dep versions:
cdk bootstrap aws://XX4312/us-east-2 —profile nextjs-cdk-test-deployer
SUCCESS!!
https://github.com/jetbridge/cdk-nextjs
In short, I added the dependencies listed
npm install -D esbuild@0.17.16 cdk-nextjs-standalone
Then modified the sample-app CDK stack as such:
cdk synth CdkStack —profile nextjs-cdk-test-deployer
I just modified the default (otherwise unmodified)
bin/cdk.ts
file produced from thecdk init sample-app ..
to include a region (and I also added my account number:cdk synth CdkStack —profile nextjs-cdk-test-deployer
I’m not sure why, but when I ran
cdk deploy
I got an error, even though I already rancdk bootstrap — profile …
- runningcdk bootstrap
againcdk bootstrap —profile nextjs-cdk-test-deployer
did show that one region was bootstrapped, but not another. So perhaps there is some sort of multi-region (something?) that is required by the Next.js cdk setup? Note below “no changes” for one region but not the other.cdk deploy CdkStack —profile nextjs-cdk-test-deployer
This “worked”, deploy kicked off (prompted to confirm security changes and such, but kicked off as expected).
I’m not aware of any other way to get the cloud front distribution domain name, so I just logged into the AWS console to get the domain, in my case
Note that the
favicon.ico
request also resulted in a 500E.g.
The other Lambda logs didn’t seem to have anything that looked like errors
The text was updated successfully, but these errors were encountered: