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

Update aws and multer #474

Merged
merged 5 commits into from
Nov 21, 2023

Conversation

duggalsu
Copy link
Contributor

@duggalsu duggalsu commented Nov 7, 2023

@duggalsu duggalsu requested a review from dennyabrain November 7, 2023 09:50
@ghost
Copy link

ghost commented Nov 7, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@aatmanvaidya
Copy link
Collaborator

aatmanvaidya commented Nov 13, 2023

Hello @duggalsu , I review the PR, I am getting an error

To review this PR

  1. I removed the node_modules and then re-installed the packages by doing npm install
  2. I added the AWS keys and bucket name to the env file.
  3. Then I started the backend server by doing docker-compose up
  4. Then , I opened Postman, in the form-data I added a png file called screenshot.png
  5. After that when I send a request to http://localhost:3000/archive

I am getting this error

api-server                  | Error: Region is missing
api-server                  |     at default (/app/node_modules/@smithy/config-resolver/dist-cjs/regionConfig/config.js:10:15)
api-server                  |     at /app/node_modules/@smithy/node-config-provider/dist-cjs/fromStatic.js:6:83
api-server                  |     at /app/node_modules/@smithy/property-provider/dist-cjs/chain.js:12:39
api-server                  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
api-server                  |     at async coalesceProvider (/app/node_modules/@smithy/property-provider/dist-cjs/memoize.js:14:24)
api-server                  |     at async /app/node_modules/@smithy/property-provider/dist-cjs/memoize.js:26:28
api-server                  |     at async region (/app/node_modules/@smithy/config-resolver/dist-cjs/regionConfig/resolveRegionConfig.js:17:36)
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/region-redirect-endpoint-middleware.js:6:32
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/region-redirect-middleware.js:9:20
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:7:26

To fix this error, we have to add REGION in our AWS config file I think

- Updated axios version
- Pinned auto-added buffer package required by parcel

# Conflicts:
#	browser-extension/plugin/package-lock.json
#	browser-extension/plugin/package.json
# Conflicts:
#	browser-extension/plugin/package-lock.json
- Updated AWS SDK for JS packages
- Updated multer-s3 package
- Migrated AWS code to use current package version
- Updated aws-sdk client-s3
- Updated aws-sdk client-sesv2
@duggalsu duggalsu force-pushed the update_aws_and_multer branch from 9d8bde3 to acdc5ab Compare November 14, 2023 06:18
@duggalsu
Copy link
Contributor Author

Hey @aatmanvaidya , since the aws-sdk is now modularized and updated with breaking changes, I used aws-sdk-js-codemod for automatic migration from v2 to v3. This removed the AWS.config.update() and added the region directly to const sendPromise = new SES() in the email.js file. The links you mentioned are for the older aws-sdk version. I will also have to troubleshoot this to see how to make this work.

I have also updated the version again (non-breaking changes).

@aatmanvaidya
Copy link
Collaborator

okay so should I review this PR now?

@duggalsu
Copy link
Contributor Author

The new version update would not resolve this. Either of us will still have to troubleshoot this issue and how to resolve it using the latest AWS SDK

@aatmanvaidya aatmanvaidya added the level:ticket An issue that describes a ticket (initiative>feature>ticket) label Nov 15, 2023
@dennyabrain
Copy link
Contributor

folks, did you try setting the region ? try setting the environment variable AWS_REGION to "ap-south-1". so something like AWS_REGION=ap-south-1 in the env file.

@aatmanvaidya
Copy link
Collaborator

no will test this out

@aatmanvaidya
Copy link
Collaborator

@dennyabrain that worked, the region error has gone
but now I am getting tihs error

api-server                  | AccessDenied: Access Denied
api-server                  |     at throwDefaultError (/app/node_modules/@smithy/smithy-client/dist-cjs/default-error-handler.js:8:22)
api-server                  |     at /app/node_modules/@smithy/smithy-client/dist-cjs/default-error-handler.js:18:39
api-server                  |     at de_PutObjectCommandError (/app/node_modules/@aws-sdk/client-s3/dist-cjs/protocols/Aws_restXml.js:5721:12)
api-server                  |     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
api-server                  |     at async /app/node_modules/@smithy/middleware-serde/dist-cjs/deserializerMiddleware.js:7:24
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-signing/dist-cjs/awsAuthMiddleware.js:14:20
api-server                  |     at async /app/node_modules/@smithy/middleware-retry/dist-cjs/retryMiddleware.js:27:46
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-flexible-checksums/dist-cjs/flexibleChecksumsMiddleware.js:63:20
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/region-redirect-endpoint-middleware.js:14:24
api-server                  |     at async /app/node_modules/@aws-sdk/middleware-sdk-s3/dist-cjs/region-redirect-middleware.js:9:20

I might need the access via IAM in AWS

Copy link
Contributor

@dennyabrain dennyabrain left a comment

Choose a reason for hiding this comment

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

Thanks @duggalsu and @aatmanvaidya
I noticed that the IAM permissions on aws were not sufficient for this user to upload files on ogbv-plugin-dev.tattle.co.in. I made the changes and now it works. We're good to merge this.

@dennyabrain
Copy link
Contributor

@duggalsu the existing conflicts are related to package and package-lock.json. I think you'll be best to resolve those.

@duggalsu duggalsu merged commit db1ec1e into tattle-made:development Nov 21, 2023
duggalsu added a commit to duggalsu/Uli that referenced this pull request Nov 22, 2023
* Update axios
- Updated axios version
- Pinned auto-added buffer package required by parcel

# Conflicts:
#	browser-extension/plugin/package-lock.json
#	browser-extension/plugin/package.json

* - Recreated package lock file

# Conflicts:
#	browser-extension/plugin/package-lock.json

* Update aws-sdk and multer-s3
- Updated AWS SDK for JS packages
- Updated multer-s3 package
- Migrated AWS code to use current package version

* Update AWS SDK modules
- Updated aws-sdk client-s3
- Updated aws-sdk client-sesv2
duggalsu added a commit that referenced this pull request Nov 29, 2023
* Update axios
- Updated axios version
- Pinned auto-added buffer package required by parcel

# Conflicts:
#	browser-extension/plugin/package-lock.json
#	browser-extension/plugin/package.json

* - Recreated package lock file

# Conflicts:
#	browser-extension/plugin/package-lock.json

* Update aws-sdk and multer-s3
- Updated AWS SDK for JS packages
- Updated multer-s3 package
- Migrated AWS code to use current package version

* Update AWS SDK modules
- Updated aws-sdk client-s3
- Updated aws-sdk client-sesv2
@duggalsu duggalsu deleted the update_aws_and_multer branch November 29, 2023 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level:ticket An issue that describes a ticket (initiative>feature>ticket)
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants