Rs hotfix/fix corrupt image uploads #7804

RileySeaburg · 2024-07-23T17:33:58Z

Summary

This PR refactors the file upload process to address issues with image uploads and file processing. It replaces the gulp-awspublish plugin with a custom TypeScript-based upload module using AWS SDK v3, while maintaining Gulp task integration for workflow consistency.

Solution

This PR implements the following changes:

Removes the gulp-awspublish plugin and replaces it with a custom TypeScript module (config/typescript/upload.ts) using AWS SDK v3 for file uploads.
Maintains separate functions for uploading images and files, preserving the existing structure.
Implements a cleanup function to remove temporary working directories after successful uploads.
Updates the Gulpfile to integrate the new TypeScript-based upload module within the existing Gulp task structure.

This approach was chosen to:

Improve type safety and catch potential errors at compile-time by using TypeScript.
Modernize the upload process by utilizing the latest AWS SDK v3, removing the dependency on the outdated gulp-awspublish plugin.
Maintain the existing Gulp-based workflow for consistency and ease of integration.
Ensure proper cleanup after the upload process.

The change was implemented by creating a new TypeScript file for uploads, compiling it to JavaScript, and integrating it into the existing Gulp tasks. This allows us to have more control over the upload process while keeping the familiar Gulp task structure.

How To Test

Install dependencies: npm install
Place test images in content/uploads/_inbox/__add image or static files to this folder__
Run the Gulp upload task: npx gulp upload
Verify that files are uploaded to the S3 bucket and local directories are cleaned up

Dependency updates

Dependency

Dev Dependency name	Previous version	New version
@aws-sdk/client-s3	--	^3.614.0
dotenv	--	^16.3.1
mime-types	--	^2.1.35

Dev Dependency

Dev Dependency name	Previous version	New version
@typescript-eslint/eslint-plugin	--	^5.x.x
@typescript-eslint/parser	--	^5.x.x
@types/mime-types	--	^2.1.4
typescript	--	^4.x.x
gulp-awspublish	^8.0.0	Removed

Script updates

Modified lint:js and lint:js:fix to include TypeScript files
Modified prettier:js and prettier:js:fix to include TypeScript files
Added build script to compile TypeScript
Added type-check script to run TypeScript's type checker

Notes

TypeScript is now a local dependency; use npm run build to compile.
ESLint configuration has been updated for TypeScript in .eslintrc.yaml.
Run npm run build, npm run lint:js, and npm run type-check before submitting PR to ensure code quality.

github-actions · 2024-07-23T17:34:46Z

🔍 Preview in Federalist

…com/GSA/digitalgov.gov into rs-hotfix/fix-corrupt-image-uploads

nick-mon1

What I Tested

Tested all file types and all steps to get a successful upload.

name	type	upload	cleanup	valid public file
test-j-image-upload-7-25-2024.jpg	jpg	pass	pass	pass
test-p-image-upload-7-25-2024.png	png	pass	pass	pass
test-document-upload-7-25-2024.pdf	pdf	pass	pass	pass
test-powerpoint-upload-7-25-2024.pptx	pptx	pass	pass	pass
test-spreadsheet-upload-7-25-2024.xlsx	xlsx	pass	pass	pass

Note

I did notice in my test file names that when I used test for the pdf it would rename from test-document-upload-7-25-2024.pdf to test.document-upload-7-25-2024.pdf. Just something to keep note of.

Suggestions

I've added some jsdocs for the functions in the upload.ts (f4e50c9)

Questions

Can we add a linting and formatting step to the package.json for the typescript? Or is that handled when running tsc?

By adding another dependency with typescript, what are the benefits and some of the risks with having to support it for the uploading process over gulp?

If an image error occurs in the future, what would we need to know about the typescript code to address?

RileySeaburg · 2024-07-25T22:18:41Z

What I Tested

Tested all file types and all steps to get a successful upload.

| name | type | upload | cleanup | valid public file |

| -------------------------------------- | ---- | ------ | ------- | ----------------- |

| test-j-image-upload-7-25-2024.jpg | jpg | pass | pass | pass |

| test-p-image-upload-7-25-2024.png | png | pass | pass | pass |

| test-document-upload-7-25-2024.pdf | pdf | pass | pass | pass |

| test-powerpoint-upload-7-25-2024.pptx | pptx | pass | pass | pass |

| test-spreadsheet-upload-7-25-2024.xlsx | xlsx | pass | pass | pass |

[!NOTE]

I did notice in my test file names that when I used test for the pdf it would rename from test-document-upload-7-25-2024.pdf to test.document-upload-7-25-2024.pdf. Just something to keep note of.

Suggestions

I've added some jsdocs for the functions in the upload.ts (f4e50c9)

Questions

Can we add a linting and formatting step to the package.json for the typescript? Or is that handled when running tsc?

By adding another dependency with typescript, what are the benefits and some of the risks with having to support it for the uploading process over gulp?

If an image error occurs in the future, what would we need to know about the typescript code to address?

@nick-mon1

This is not robust code to handle every edge case, merely a search for a peer review to ensure there are no obvious errors.

The key thing to remember about TypeScript is, JavaScript is valid TypeScript.

JsDocs and ESLint can assist in ensuring that formatting and documentation are handled.

I can add a commit that further integrates typescript into the tooling tomorrow.

Overall this does not complicate the gulp upload process.

They only thing it does is solve the data race issue by using Node.js to upload the files and images.

This could just as easily be written with vanilla.js.

nick-mon1

@RileySeaburg Sounds good, please update the package.json with any formatting and linting commands to prevent build errors.

It would be preferable to use vanillia.js because it is one less dependency to support but it is best to get this unblocked now.

With that caveat in mind, thanks for getting this fixed.

RileySeaburg · 2024-07-26T19:48:52Z

@nick-mon1 Idk why it said I dismissed you request.

I addressed them all here.
JSDoc and ESLint support enabled.

I added a settings.json for workspace ESLint Highlighting as well.

RileySeaburg · 2024-07-26T19:58:13Z

I simplified the testing instructions since compiling typescript is not necessary.

RileySeaburg · 2024-07-26T20:10:01Z

package.json

@@ -7,24 +7,26 @@
    "federalist": "export NODE_ENV=production && gulp buildAssets",
    "hugo": "export $(grep -v '^#' .env | xargs) && hugo serve --bind='0.0.0.0'",
    "lint:json": "find public/**/v1/json/index.html -print0 | xargs -0I {} jsonlint '{}'",
-    "lint:js": "eslint './themes/digital.gov/src/js/*.js'",


@nick-mon1 ESlint was originally not linting any JS in ./config/gulp

Could explain why it was so difficult to work with.

I'm going to update this to lint the gulp files.

I know I'm asking for trouble, but it only makes sense to lint all the JS into the project.

mejiaj

Thanks for your hard work on this. I was able to upload files successfully without any corruption. I like the improvements to linting (JSDoc) and documentation too.

Added minor comments and questions. The biggest one being the addition of typescript.

Tested

Uploaded powerpoint without corruption
Uploaded PNG successfully ¹
Uploaded JPEG successfully ²

Build log

[14:24:06] Using gulpfile ~/web/digitalgov.gov/gulpfile.js
[14:24:06] Starting 'upload'...
[14:24:06] Starting 'fileTidy'...
Moving file from content/uploads/_inbox/TEST-USWDS Monthly Call June 2024.pptx to content/uploads/_working-files/to-process/test-uswds-monthly-call-june-2024.pptx
Moving file from content/uploads/_inbox/TEST-dg-7804.png to content/uploads/_working-images/to-process/test-dg-7804.png
Moving file from content/uploads/_inbox/TEST-ooh.png to content/uploads/_working-images/to-process/test-ooh.png
[14:24:06] Finished 'fileTidy' after 2.24 ms
[14:24:06] Starting 'cleanInbox'...
[14:24:06] Finished 'cleanInbox' after 4.16 ms
[14:24:06] Starting 'writeDataFile'...
file is written
[14:24:06] Finished 'writeDataFile' after 64 ms
[14:24:06] Starting 'processImages'...
[14:24:06] Finished 'processImages' after 2.77 ms
[14:24:06] Starting 'removeProcessedImage'...
file is written
file is written
[14:24:06] Finished 'removeProcessedImage' after 37 ms
[14:24:06] Starting 'uploadTask'...
Error processing variant /Users/jmejia-a/web/digitalgov.gov/content/uploads/_working-images/to-process/test-ooh.png: Error: /Users/jmejia-a/web/digitalgov.gov/content/uploads/_working-images/to-process/test-ooh.png: unable to open for read
unix error: No such file or directory
/Users/jmejia-a/web/digitalgov.gov/content/uploads/_working-images/to-process/test-ooh.png: unable to open for read
unix error: No such file or directory
pngload: stream error
    at Sharp.toFile (/Users/jmejia-a/web/digitalgov.gov/node_modules/sharp/lib/output.js:89:19)
    at createResponsiveVariant (/Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:90:6)
    at /Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:110:5
    at Array.forEach (<anonymous>)
    at processImageVariants (/Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:109:34)
    at /Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:139:11
    at Array.map (<anonymous>)
    at /Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:135:41
    at FSReqCallback.oncomplete (node:fs:191:23)
    at FSReqCallback.callbackTrampoline (node:internal/async_hooks:130:17)
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w1200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w400.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w800.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w1200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w400.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w800.png
File uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/static/test-uswds-monthly-call-june-2024.pptx
All uploads completed successfully.
Cleanup completed
[14:24:10] Finished 'uploadTask' after 4.41 s
[14:24:10] Finished 'upload' after 4.52 s

Files tested

Additional chores

We'll need to delete all test files on S3 bucket.

Uploaded a small PNG that triggered a build error, but uploaded successfully. ↩
Image was converted to PNG. Is this intentional? PNG files are larger and can be worse quality than JPEG. ↩

config/gulp/readme.md

config/typescript/upload.ts

package.json

config/typescript/upload.ts

RileySeaburg · 2024-07-29T17:15:48Z

Thanks for your hard work on this. I was able to upload files successfully without any corruption. I like the improvements to linting (JSDoc) and documentation too.

Added minor comments and questions. The biggest one being the addition of typescript.

Tested

Uploaded powerpoint without corruption
Uploaded PNG successfully 1
Uploaded JPEG successfully 2

Build log

[14:24:06] Using gulpfile ~/web/digitalgov.gov/gulpfile.js
[14:24:06] Starting 'upload'...
[14:24:06] Starting 'fileTidy'...
Moving file from content/uploads/_inbox/TEST-USWDS Monthly Call June 2024.pptx to content/uploads/_working-files/to-process/test-uswds-monthly-call-june-2024.pptx
Moving file from content/uploads/_inbox/TEST-dg-7804.png to content/uploads/_working-images/to-process/test-dg-7804.png
Moving file from content/uploads/_inbox/TEST-ooh.png to content/uploads/_working-images/to-process/test-ooh.png
[14:24:06] Finished 'fileTidy' after 2.24 ms
[14:24:06] Starting 'cleanInbox'...
[14:24:06] Finished 'cleanInbox' after 4.16 ms
[14:24:06] Starting 'writeDataFile'...
file is written
[14:24:06] Finished 'writeDataFile' after 64 ms
[14:24:06] Starting 'processImages'...
[14:24:06] Finished 'processImages' after 2.77 ms
[14:24:06] Starting 'removeProcessedImage'...
file is written
file is written
[14:24:06] Finished 'removeProcessedImage' after 37 ms
[14:24:06] Starting 'uploadTask'...
Error processing variant /Users/jmejia-a/web/digitalgov.gov/content/uploads/_working-images/to-process/test-ooh.png: Error: /Users/jmejia-a/web/digitalgov.gov/content/uploads/_working-images/to-process/test-ooh.png: unable to open for read
unix error: No such file or directory
/Users/jmejia-a/web/digitalgov.gov/content/uploads/_working-images/to-process/test-ooh.png: unable to open for read
unix error: No such file or directory
pngload: stream error
    at Sharp.toFile (/Users/jmejia-a/web/digitalgov.gov/node_modules/sharp/lib/output.js:89:19)
    at createResponsiveVariant (/Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:90:6)
    at /Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:110:5
    at Array.forEach (<anonymous>)
    at processImageVariants (/Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:109:34)
    at /Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:139:11
    at Array.map (<anonymous>)
    at /Users/jmejia-a/web/digitalgov.gov/config/gulp/file-process.js:135:41
    at FSReqCallback.oncomplete (node:fs:191:23)
    at FSReqCallback.callbackTrampoline (node:internal/async_hooks:130:17)
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w1200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w400.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-dg-7804_w800.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w1200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w200.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w400.png
Image uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/test-ooh_w800.png
File uploaded successfully. Public URL: https://digitalgov.s3.amazonaws.com/static/test-uswds-monthly-call-june-2024.pptx
All uploads completed successfully.
Cleanup completed
[14:24:10] Finished 'uploadTask' after 4.41 s
[14:24:10] Finished 'upload' after 4.52 s

Files tested

Additional chores

We'll need to delete all test files on S3 bucket.

Footnotes

Uploaded a small PNG that triggered a build error, but uploaded successfully. ↩
Image was converted to PNG. Is this intentional? PNG files are larger and can be worse quality than JPEG. ↩

Addressing your footnote, this is probably an artifact from the jpg -> png. issue #7686.
I will address this as well.

@mejiaj

RileySeaburg · 2024-07-29T17:43:43Z

I just found out Gulp has typescript support.

Let's merge this for now since it's confirmed working - To fix production upload and will reconfigure in another PR and trello card to clean up the workflow.

RileySeaburg added 3 commits July 11, 2024 15:36

fix corruption issue

483fce2

fix folder delete issue

c2bb85a

Tested with word docs and jpg > png

043d71b

fix import issue

807e896

RileySeaburg added Bug Fix This fixes an actual bug Content: Images labels Jul 23, 2024

RileySeaburg self-assigned this Jul 23, 2024

RileySeaburg requested review from nick-mon1 and mejiaj July 23, 2024 17:45

RileySeaburg and others added 4 commits July 23, 2024 12:45

Merge branch 'main' into rs-hotfix/fix-corrupt-image-uploads

d06fbcf

Fix mime type issue

5678180

Merge branch 'main' into rs-hotfix/fix-corrupt-image-uploads

d3d3f14

Merge branch 'main' into rs-hotfix/fix-corrupt-image-uploads

bb5fe65

mejiaj requested a review from clmedders July 25, 2024 13:54

RileySeaburg and others added 3 commits July 25, 2024 10:54

Merge branch 'main' into rs-hotfix/fix-corrupt-image-uploads

953d153

Fixed some formatting and added jsdocs comments to functions

f4e50c9

Merge branch 'rs-hotfix/fix-corrupt-image-uploads' of https://github.…

1522588

…com/GSA/digitalgov.gov into rs-hotfix/fix-corrupt-image-uploads

nick-mon1 requested changes Jul 25, 2024

View reviewed changes

Merge branch 'main' into rs-hotfix/fix-corrupt-image-uploads

b0fa759

nick-mon1 self-requested a review July 26, 2024 18:58

nick-mon1 previously approved these changes Jul 26, 2024

View reviewed changes

RileySeaburg added 5 commits July 26, 2024 14:33

Add updated rules for eslint

668cb78

Enable eslint highliting in editor

eb75183

Add eslint related dependenceis for typescript

8ab9a1e

fix minor eslint issues

9f578e3

fix minor eslint issues

22504f3

RileySeaburg dismissed nick-mon1’s stale review via 22504f3 July 26, 2024 19:37

RileySeaburg commented Jul 26, 2024

View reviewed changes

mejiaj suggested changes Jul 29, 2024

View reviewed changes

Merge branch 'main' into rs-hotfix/fix-corrupt-image-uploads

49f6be9

mejiaj approved these changes Jul 29, 2024

View reviewed changes

RileySeaburg merged commit 12387d7 into main Jul 29, 2024
8 checks passed

RileySeaburg deleted the rs-hotfix/fix-corrupt-image-uploads branch July 29, 2024 19:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rs hotfix/fix corrupt image uploads #7804

Rs hotfix/fix corrupt image uploads #7804

RileySeaburg commented Jul 23, 2024 •

edited

Loading

github-actions bot commented Jul 23, 2024

nick-mon1 left a comment

RileySeaburg commented Jul 25, 2024

What I Tested

Suggestions

Questions

nick-mon1 left a comment

RileySeaburg commented Jul 26, 2024

RileySeaburg commented Jul 26, 2024

RileySeaburg Jul 26, 2024

RileySeaburg Jul 26, 2024

RileySeaburg Jul 26, 2024

mejiaj left a comment

RileySeaburg commented Jul 29, 2024 •

edited

Loading

Tested

Files tested

Additional chores

Footnotes

RileySeaburg commented Jul 29, 2024

Rs hotfix/fix corrupt image uploads #7804

Rs hotfix/fix corrupt image uploads #7804

Conversation

RileySeaburg commented Jul 23, 2024 • edited Loading

Summary

Solution

How To Test

Dependency updates

Dependency

Dev Dependency

Script updates

Notes

github-actions bot commented Jul 23, 2024

nick-mon1 left a comment

Choose a reason for hiding this comment

What I Tested

Suggestions

Questions

RileySeaburg commented Jul 25, 2024

What I Tested

Suggestions

Questions

nick-mon1 left a comment

Choose a reason for hiding this comment

RileySeaburg commented Jul 26, 2024

RileySeaburg commented Jul 26, 2024

RileySeaburg Jul 26, 2024

Choose a reason for hiding this comment

RileySeaburg Jul 26, 2024

Choose a reason for hiding this comment

RileySeaburg Jul 26, 2024

Choose a reason for hiding this comment

mejiaj left a comment

Choose a reason for hiding this comment

Tested

Files tested

Additional chores

Footnotes

RileySeaburg commented Jul 29, 2024 • edited Loading

Tested

Files tested

Additional chores

Footnotes

RileySeaburg commented Jul 29, 2024

RileySeaburg commented Jul 23, 2024 •

edited

Loading

RileySeaburg commented Jul 29, 2024 •

edited

Loading