search

processing / p5.js-web-editor

The p5.js Editor is a website for creating p5.js sketches, with a focus on making coding accessible and inclusive for artists, designers, educators, beginners, and anyone else! You can create, share, or remix p5.js sketches without needing to download or configure anything.
https://editor.p5js.org
GNU Lesser General Public License v2.1
1.3k stars 1.27k forks source link

`deleteProject` handler doesn't wait for S3 deletion to complete #3023

Closed lindapaiste closed 3 weeks ago

lindapaiste commented 4 months ago

Increasing Access

It seems like we could end up with files left behind on S3, which use up resources.

Feature enhancement details

The deleteProject handler fires off an asynchronous function deleteObjectsFromS3 (via deleteFilesFromS3) but it does not wait for the S3 deletion to continue before sending an API response.

I'm not sure what actually happens here -- do those files continue to get deleted in the background or not? I'm concerned that we could wind up with "leftover" files on S3 that are no longer linked to any project.

There is no waiting here: https://github.com/processing/p5.js-web-editor/blob/6cac275429c3911d87afdcdbbdbf185092aeac29/server/controllers/project.controller/deleteProject.js#L61-L72

I noticed this thanks to a warning in my IDE image

rahulrana701 commented 4 months ago

I think for that we need to ensure that the s3 deletion process completes before sending the api response we can achieve it by modifying the delete project function to wait for the asynchronous deleteFilesFroms3 function to finish executing , for that we can use promises or asynchronous/await @lindapaiste what are your views on this , if this approach is correct , I would like to work on this issue

ash97531 commented 4 months ago

Can anyone please check where I am making a mistake in the AWS setup? I have described my error in this thread: https://github.com/processing/p5.js-web-editor/discussions/2984

rahulrana701 commented 4 months ago

@lindapaiste what is your say on this ??

letscodedanish commented 4 months ago

Problem

The warning "Promise returned from deleteObjectsFromS3 is ignored" indicates that the deleteObjectsFromS3 function is returning a promise, but its resolution is not being handled properly. This warning suggests that there might be potential issues with the asynchronous operation not being properly managed, which could lead to unexpected behavior or resource leaks.

Action to fix

To address this warning, you need to ensure that you handle the promise returned by deleteObjectsFromS3 appropriately. This typically involves using either async/await or .then() to handle the promise resolution and execution flow.

To fix the warning "Promise returned from deleteObjectsFromS3 is ignored," you can take the following actions:

1.Handle the Promise: Ensure that the Promise returned by deleteObjectsFromS3 is properly handled to avoid potential issues with asynchronous operations. We can use async/await or .then() to handle the Promise resolution and execution flow. 2.Using async/await

@lindapaiste I would love to fix this issue, please assign this to me.

PiyushChandra17 commented 1 month ago

@lindapaiste @raclim I think on the latest develop, we are already handling the promises returned from deleteFilesFromS3 using async/await. IMO async/await is much cleaner and modern way to handle promises instead of .then() and .catch(). I have just added more robust way to handle error in the catch block here