Skip to content
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

feat: deploy / adapter / AWS Lambda #48

Draft
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

aheissenberger
Copy link
Contributor

@aheissenberger aheissenberger commented Sep 25, 2024

Adapter to deploy to AWS Lambda. Wraps the hattip middleware with a AWS Lambda specific handler.
fix #30

Status with different Deployment Frameworks:

  • Serverless Framework V3
  • sst V3 [^1] [^2]
  • AWS CDK [^3]
  1. deploy is possible but dev mode fails because it always tries to bundle the application. This fails with the excluded /create-logger.mjs and cannot handle the native libs for fsevents and lightningcss
  2. I suggest to build a specific stack with dev server handling similar to [sst/ ion remix stack[(https://github.com/sst/ion/blob/b9c9760259ebcd4419dc15679ebdbc61babc517f/platform/src/components/aws/remix.ts)
  3. works, but function bundling needs to be deactivated

@aheissenberger aheissenberger marked this pull request as draft September 25, 2024 03:04
@aheissenberger aheissenberger changed the title Deploy / Adapter / AWS Lambda feat: Deploy / Adapter / AWS Lambda Sep 25, 2024
@aheissenberger
Copy link
Contributor Author

aheissenberger commented Sep 25, 2024

@lazarv

  1. it works but I always get a statusCode 404 with my test request in examples/hello-world-aws:
    node test.mjs
    It looks like the wrong current working directory. Can you give me a hint how to solve this?
    The problem was the accept: "*/*" request header. After adding text/html it worked.
  2. look at packages/react-server-adapter-aws/libs folder:
    • except for create-aws-lambda-handler.mjs everything else is generic und could be part of @lazarv/react-server
    • this should allow to add other deployment targets with existing hattip adapters

@aheissenberger
Copy link
Contributor Author

aheissenberger commented Sep 25, 2024

@lazarv the adapter is ready but I need help with these small problems:

  • How can I add define: { 'process.env.NODE_ENV': JSON.stringify(mode), }, from the deploy adapter to vite to ensure no development code is called as this fails after a build as the optimitzation has removed the required code.
  • Currently I need to explicitly set NODE_ENV=production in AWS Lambda to avoid that the react version of the client (production) differs from backend (development).
  • Each deployment framework will call the entry point from a diffenerent current working directory and some (e.g. serverless framework) have no option to change this. Currently I added the option to specify the .react-server directory in relation to cwd with environment variable OUT_DIR.

Example Error from sst dev

| Build Error ApiGatewayRouteBbovcaHandler
|  ↳ Could not resolve "../lib/dev/create-logger.mjs" .aws-lambda/output/functions/index.func/node_modules/@lazarv/react-server/server/logger.mjs:6:20

create-logger.mjs does not exist in the optimizes output/functions/index.func/node_modules folder.

when create-logger.mjsis included it fails with:

|  Build Error ApiGatewayRouteBbovcaHandler
|  ↳ No loader is configured for ".node" files: .aws-lambda/output/functions/inde
x.func/node_modules/fsevents/fsevents.node .aws-lambda/output/functions/index.fun
c/node_modules/fsevents/fsevents.js:13:23
|  ↳ Could not resolve "lightningcss" .aws-lambda/output/functions/index.func/nod
e_modules/vite/dist/node/chunks/dep-DXWVQosX.js:62529:59
|  Build Error ApiGatewayRouteBbovcaHandler
|  ↳ No loader is configured for ".node" files: .aws-lambda/output/functions/inde
x.func/node_modules/fsevents/fsevents.node .aws-lambda/output/functions/index.fun
c/node_modules/fsevents/fsevents.js:13:23
|  ↳ Could not resolve "lightningcss" .aws-lambda/output/functions/index.func/nod
e_modules/vite/dist/node/chunks/dep-DXWVQosX.js:62529:59
|  Build Error ApiGatewayRouteBbovcaHandler
|  ↳ No loader is configured for ".node" files: .aws-lambda/output/functions/inde
x.func/node_modules/fsevents/fsevents.node .aws-lambda/output/functions/index.fun
c/node_modules/fsevents/fsevents.js:13:23
|  ↳ Could not resolve "lightningcss" .aws-lambda/output/functions/index.func/nod
e_modules/vite/dist/node/chunks/dep-DXWVQosX.js:62529:59

@aheissenberger
Copy link
Contributor Author

The problem is not related to NODE_ENV, it's the additional bundling by ˋsstˋ which fails with imports not represented in the node_module folder. I will try to find an option to switch off bundling

@lazarv
Copy link
Owner

lazarv commented Sep 26, 2024

Hi @aheissenberger!

I will implement / refactor what you have right now in the AWS adapter to be able to just provide a function which creates a Hattip handler / middleware, so it's easier to create handlers for different platforms, like Node.js, AWS, etc. But I can't promise when I'll have time to do it.

I tried to deploy to AWS lambda using Serverless Framework v3 and v4, but neither worked. Your test run works, but there were issues with the deployments.

Using v3 it creates a handler which calls the specified function handler, but it's implemented using CommonJS which can't call ESM. With v4 I had other module resolution issues. Could you please provide a working serverless.yaml? I think it would be nice if the output would be usable straight after a build, like the Vercel adapter to deploy the app right ahead.

@aheissenberger
Copy link
Contributor Author

@lazarv I am finished and suggest to move any kind of optimization regarding configuration of deployment frameworks to its own PRs
Please have a look at the included example project which provides deployment configuration for three different deployment frameworks.

Currently your methods in core do not copy any source map files which is fine for me as I upload them directly to sentry but I suggest to add an option which allows to define if the source maps should be included in the output for the adapter.

regarding sst v3 - a stack should be created but I do not know the internals of react-server well enough to connect to the dev server as done in the stack for e.g. remix or other existing stacks directly provided by sst:
https://github.com/sst/ion/blob/b9c9760259ebcd4419dc15679ebdbc61babc517f/platform/src/components/aws/remix.ts

slow startup time: Have you ever looked at the startup time of your server? The current startup time for a lambda is Init Duration: 756.41 ms which is quit high. Further request for the example app are Duration: 37.10 ms which is ok.

@aheissenberger aheissenberger changed the title feat: Deploy / Adapter / AWS Lambda feat: deploy / adapter / AWS Lambda Sep 26, 2024
@lazarv
Copy link
Owner

lazarv commented Sep 27, 2024

@aheissenberger this is great! As soon as you think this is ready to go, just mark it ready for review and it's good to go!

Please have a look at the included example project which provides deployment configuration for three different deployment frameworks.

I went through all of them, everything works! My only concern is about the SST deployment as it doesn't use CloudFront to serve all the static files from an S3 bucket, but every request is handled by the lambda, which is very suboptimal.

Currently your methods in core do not copy any source map files which is fine for me as I upload them directly to sentry but I suggest to add an option which allows to define if the source maps should be included in the output for the adapter.

The build can get the information from the configuration that it's needed to copy source maps or not, an enhancement for later for sure.

regarding sst v3 - a stack should be created but I do not know the internals of react-server well enough to connect to the dev server as done in the stack for e.g. remix or other existing stacks directly provided by sst

I think this is a future task and should be implemented at SST like in the case of Remix or Next.js with support from react-server's side.

Have you ever looked at the startup time of your server?

This is a performance issue around loading the configuration files. If loadConfig just return early with an empty configuration object it shaves off ~150ms. I will investigate this further.

function bundling needs to be deactivated

I think that bundling is almost impossible right now because how client components and server actions are resolved and imported on-demand. It could result in shrinking the size a bit regarding the dependencies in node_modules, but not much on the app code side which is already minified during build.

@aheissenberger
Copy link
Contributor Author

I have added a first version of a sst stack for react-server which supports s3 for static assets.

Last step will be to add optional lambda streaming.

@aheissenberger
Copy link
Contributor Author

@lazarv any idea why the @esbuild is included in the examples/hello-world-aws/.aws-lambda/output/functions/index.func/node_modules?

Here is a list sorted by size:
10M @esbuild
1,4M react-dom
720K parse5
516K react-server-dom-webpack
304K node-fetch-native
180K react
176K entities
172K @lazarv
132K esbuild
112K fast-glob
112K @nodelib
104K @Hattip
72K picomatch
52K mime
40K braces
32K react-property
20K style-to-object
20K micromatch
12K to-regex-range
12K style-to-js
12K module-alias
12K inline-style-parser
12K fill-range
12K fastq
12K cookie
8,0K run-parallel
8,0K reusify
8,0K queue-microtask
8,0K picocolors
8,0K merge2
8,0K is-number
8,0K is-glob
8,0K is-extglob
8,0K glob-parent

@lazarv
Copy link
Owner

lazarv commented Sep 28, 2024

any idea why the esbuild is included

Again, because of configuration. That module needs special attention from my side and I'll fix the performance and dependency issues around it this weekend.

Regarding patching SST with a symlink, can you move that into the adapter package and add Pulumi as an optional peer dependency? Also that "master" file needs some cleanup, like removing unused import comment and fixing TanStack Start link. I'll check out what's needed to make dev mode work.

If you could move the instructions from the readme into the docs that would be awesome!

@aheissenberger
Copy link
Contributor Author

The symlink is only a hack to allow me to test the stack - I am not experienced with sst framework at all and the documentation on how to develop a stack outside the repository does not exist. Currently all frameworks exist directly in the sst repository which is cool in the long run but currently as things still move a little bit problematic. I have asked on discord on how to handle this case.

I will upgrade the docs to include the content of the readme.

@lazarv
Copy link
Owner

lazarv commented Sep 28, 2024

I was able to significantly reduce the startup time with changes regarding configuration and static file handling. On the main branch before the changes the docs started locally in ~265ms for me in production mode. After the changes this is down to ~72-75ms.

This configuration handling change also removes the esbuild dependency when using a deployment adapter.

You will need to change the import at https://github.com/aheissenberger/react-server/blob/feat/adapter/aws-lambda/packages/react-server-adapter-aws/libs/create-handler.mjs#L1 to use https://github.com/lazarv/react-server/blob/main/packages/react-server/config/prebuilt.mjs. This way it's loading a prebuilt version of the configuration much faster than using glob in development mode.

@aheissenberger
Copy link
Contributor Author

@lazarv I try to find a setup with AWS CDK for the AWS CloudFront & AWS Lambda Setup to handle Routing for static und dynamic routes.

I have tried the following setups:

A) static top level folder strategy (tested/ old implementation )

  • get all static top level folder from .aws-react-server/output/static
  • add them as CloudFront Behaviors pointing to a S3 bucket with all the static files.
  • Add a Cloudfront function (viewer request) to all Behaviors to rewrite directory requests to index.html
  • default behavior route points to the AWS Lambda function with the react server framework stack
  • static files and routes have a higher priority than dynamic routes

Problems:

  • all subfolder requests are forwarded to S3 - eg. one page in a sub folder is static, one page is dynamic, the dynamic page will fail
  • any root folder in public will catch all requests starting with the folder name

B) Static File Lookup-Table (implemented)

  • only the folder client/* and assets/* have catch all routes to the S3 bucket
  • all paths of static files are stored in a AWS Cloudfront KeyValueStore
  • default behavior route points to the AWS Lambda function with the react server framework stack
  • the default behavior has a AWS Cloudfront function (viewer request) which redirects to S3 origin if the current url is in the KeyValueStore
  • static files and routes have a higher priority than dynamic routes

Problems:

  • Cloudfront Cache Miss for all static resources (I am investigating this currently)
  • all static file paths need to be uploaded to Cloudfront KV Store

C) Edge Routing (not impemented)

  • only the folder client and assets have catch all routes to the S3 bucket
  • default behavior route points to the AWS Lambda function with the react server framework stack
  • the default behavior has a AWS Cloudfront function (viewer request) know the routing configuration and will forward all routes not matched to the static S3 bucket
  • dynamic router have a higher priority than static routes

Problems:

  • I would need a function which provides me with the app routing data and a routing function which runs in a AWS Cloudfront function (no Node environment ) with a 10KB code size limit. Routingdata could be loaded from KV store.
  • I need an additional AWS Cloudfront function to handle links which do not exist on S3

Generell Problems

  • the extension .x-component has no known content type - to avoid the content type binary/octet-stream which works in the browser but will not be compressed by Cloudfront. Cloudfront supports only compression for these content types. Solving this requires to sync the static twice with different include/exclude filters to set a custom content type for the RSC content. With sst V3 I need to patch one of their internal libraries to add support for the extension and needed content type.
  • currently the folder in .aws-react-server/output/static are a mix of folders with files which have a cache breaker e.g. assets and client, folder which get created by static page routes, and folder which are copied from the public folder. A api which allows me to know the type of the assets would help to optimize upload, defining the http client side caching and setting the path for invalidation.

Solution C) is a long term goal but the problem exists with other cloud providers. Currently I will try to fix the existing problems with B).

The solution with sst V3 is finished, but there are some limits which are to deep in the sst stack to be fixed by me and I had some situations where the deployment broke without an option to recover what made me plan to stay with the proven AWS Cloudformation Stack deployment solution.

Sample App:
sst: https://d3bvhvpyh1vxs9.cloudfront.net/ (solution A)
AWS CDK: https://d53oahuh7w7td.cloudfront.net/ (solution B)

@lazarv
Copy link
Owner

lazarv commented Dec 17, 2024

I would suggest the following approach:

  • route all GET/HEAD to S3 and skip to using the Lambda function for any other HTTP methods
  • when URL matches /(@[^.]+\.)?(rsc|remote)\.x-component$/ you know, that this is an RSC request and in this case you can append the Content-Type header as text/x-component; charset=utf-8
  • in a Lambda@Edge function, check the response status code from S3
  • if S3 returns with 404, rewrite URL to ${pathname}/index.html
  • if S3 returns 404 again and the URL ends with index.html, then this request should be dynamic
  • use the Lambda function where the dynamic react-server handler runs by forwarding the request to an API Gateway by rewriting the domain

Right now the routing manifest for the file-system based router only exists as a virtual module at @lazarv/react-server/file-router/manifest and you can check out how I used this at https://github.com/lazarv/react-server/blob/main/packages/react-server/lib/plugins/file-router/entrypoint.jsx#L4-L8. But as this is only a virtual module, it's not really possible to access it in the adapter. Another problem with this is that matching the routes would mean to use some internal framework API as the framework supports custom matchers.

Also keep in mind that while it seems that the file-system based router will be the most used, it's not the only way to create apps with react-server. When you have a single entrypoint handling all the rendering, you can still specify static routes in react-server.config.mjs dynamically. In this case, there's no routing manifest, but any kind of static files exported at build time. So relying on a manifest does not cover all the use cases supported by the framework.

Your better choice is to use the list of static files instead, as you have it using files.static(), etc. Check out https://react-server.dev/deploy/api#adapter-handler. Then you can include this list close to Cloudfront and you can use this to decide whether to serve from S3 or using the Lambda, but this has limitations. I would still use the approach I described above. Because of Cloudfront caching, the routing logic between S3 and Lambda would run only a few times, and then all the static files would be served properly cached.

Please let me know if I missed something!

@aheissenberger
Copy link
Contributor Author

I do not like Lambda@Edge functions as they require a multi region deployment but I will give this variation a try.
I should look more into your great documentation before asking for help 😊

https://d53oahuh7w7td.cloudfront.net/ (Variation B) is now working with caching (overcaching) in cloudfront.

@aheissenberger
Copy link
Contributor Author

@lazarv I have different deployment commands based on the toolkit (cdk, sst, serverless framework) used to deploy. Currently the {deploy: ".."} only expects a string and is called before calling the deployment adapter.

I would either need this optional to be a function called with the adapterOptions or being called after the deployment adapter process is finished to show the correct deployment command.

lazarv added a commit that referenced this pull request Dec 19, 2024
Fixes handling the `deploy` CLI argument in the build action adapter
handler addressing #101.

Adds handling `deploy` property of an adapter implementation when the
value of `deploy` is a function. See more in the docs at
https://react-server.dev/deploy/api#define-the-adapter-handler.
Implements feature request at
#48 (comment)
@lazarv
Copy link
Owner

lazarv commented Dec 19, 2024

Now you can use a function as the deploy definition to make it dynamic, more about this at https://react-server.dev/deploy/api#define-the-adapter-handler

I think you can now remove the temporary file for framework detection, you'll also get the return value of the handler function in deploy beside the adapterOptions and the CLI options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hosting Adapter AWS Lambda
2 participants