Skip to content
This repository has been archived by the owner on Sep 12, 2023. It is now read-only.

Create CR endpoint with Middy.js #414

Merged
merged 13 commits into from
Jan 13, 2022
Merged

Conversation

jamescd18
Copy link
Member

Okay so this one is kinda interesting. I'm learning just how little I know / knew about AWS Lambda. I'm also realizing how long a single create endpoint can get compared to the dinky simple get endpoints we started with. It really shows how naive I was in thinking that endpoints would be small enough to bundle together with route matching.

This honestly probably is the better way to architect our backend. It just feels so much cleaner. And probably more testable honestly, though I haven't given that a shot yet.

This likely also puts us in a much better spot for a future in which we might consider migrating to full-fledged AWS instead of riding off of Netlify, but that future scares me given how little DevSecOps knowledge our team has at the moment.

@jamescd18 jamescd18 added the back-end Database, REST API, or server-side issues label Jan 2, 2022
@jamescd18 jamescd18 requested a review from acd124 January 2, 2022 04:19
@jamescd18 jamescd18 self-assigned this Jan 2, 2022
@jamescd18 jamescd18 linked an issue Jan 2, 2022 that may be closed by this pull request
16 tasks
@jamescd18
Copy link
Member Author

jamescd18 commented Jan 2, 2022

Okay so I think this PR is actually basically in a ready state to merge if we wanted to, but here's the loose ends:

  • Testing for this new design pattern / architecture
  • Updating back-end route strings (aka re-linking the front-end to this new back-end design)
  • Test if this will actually work in production on Netlify

As soon as we moved away from hard-coded data in the back-end to linking to the database, we basically stopped testing the back-end because I couldn't get proper test mocking to work to remove the Prisma client. I haven't thought about it yet, but I'm kinda guessing that this new pattern may be easier to test.

Copy link
Contributor

@acd124 acd124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the way this is looking, there's certainly a lot more to look into on this front, but validation looks a lot simpler now.

src/backend/functions/change-request-new.ts Outdated Show resolved Hide resolved
src/backend/functions/change-request-new.ts Outdated Show resolved Hide resolved
@jamescd18
Copy link
Member Author

Update: I did a test deploy and the Netlify Deploy Preview build failed so I put in a request to ITS to request those logs so we can determine what's wrong. It's possible it's just a lint error, or it could be something worse.

@jamescd18
Copy link
Member Author

As recommended by @acd124, it looks like changing to the new esbuild node bundler in the netlify.toml fixed the failing deploy preview.

https://answers.netlify.com/t/error-could-not-find-encoding-module-in-file-netlify-function/2259/19

@jamescd18
Copy link
Member Author

yet now I get another error that says utils js file exports is not found. I suspect it might have something to do with the new node bundler since that enables esmodule syntax natively supported by the functions when originally we set up the utils package (for usage with the functions) to use commonJS module syntax. alas, this switch did not fix it so I am awaiting ITS' response for logs again

Screen Shot 2022-01-10 at 5 26 42 PM

Screen Shot 2022-01-10 at 5 26 56 PM

@acd124
Copy link
Contributor

acd124 commented Jan 10, 2022

https://answers.netlify.com/t/lambda-function-es-module-support/30673/16 The answer to the problem is likely somewhere in this thread, though we'll have to wait for the logs to confirm what the right solution is. I believe the final messages in the thread say that using esbuild bundler does work, but we may want to specify using node 14.

@willfarrell
Copy link

Hi, hope you're enjoying using Middy. Saw you linked to a middy issues, so thought I'd poke around. Wanted to let you know that v3 will have a http-router if that helps (middyjs/middy#782). Also it might be worth checking out https://github.com/willfarrell/middy-ajv if you plan to have a lot of cold starts.

@acd124
Copy link
Contributor

acd124 commented Jan 11, 2022

Following research into https://answers.netlify.com/t/local-npm-dependency-in-package-json-not-installing/22291/14 the current best solution for making sure utils is properly installed is to include the preinstall script in the build script. Looking into the underlying cause shows that it is an issue with netlify's caching and there is an issue open netlify/build-image#523 about it. There is also a solution pending that could fix it as shown in netlify/build-image#513 more research might be done to see if there are useful tools or changes to be learned here about our deployment process, but for now the main goal is to get the app running.

@jamescd18
Copy link
Member Author

Thank you @willfarrell! Really excited to see v3 come out!

@jamescd18
Copy link
Member Author

Okay! I looks like adding npm ci to run before the build script fixed it. Now we just need to get the login page Google login button to work properly too, so I'm waiting for ITS to rename the env variable.

@jamescd18
Copy link
Member Author

The front-end doesn't work because of the env variable naming, but I can still send requests to the back-end via Thunder Client (VSCode integrated Postman). Sending requests to this new endpoint properly trigger "event object failed validation" so I'd like to believe that it's actually fully working. Obviously we still need to do stuff for the production database and stuff, but this is a huge step forward.

@jamescd18 jamescd18 marked this pull request as ready for review January 13, 2022 07:54
@jamescd18
Copy link
Member Author

SUCCESSFUL EXECUTION OF THE CREATE ENDPOINT!!!!!!!!!

Screen Shot 2022-01-13 at 3 15 20 AM

Screen Shot 2022-01-13 at 3 16 02 AM

@jamescd18 jamescd18 merged commit d64b09a into main Jan 13, 2022
@jamescd18 jamescd18 deleted the #412-create-cr-backend-middy branch January 13, 2022 08:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
back-end Database, REST API, or server-side issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Change Requests - Backend Create Endpoint
3 participants