TimMikeladze / next-upload

🗃️ Turn-key solution for signed & secure file-uploads to an S3 compliant storage service such as R2, AWS, or Minio. Built for Next.js. Generates signed URLs for uploading files directly to your storage service and optionally integrates with a database to store additional metadata about your files.
MIT License
83 stars 4 forks source link

Why eat the possible errors in the upload ? #66

Open vegandiet705 opened 5 days ago

vegandiet705 commented 5 days ago

Hello,

In the file src/react/useNextUpload.tsx, in the upload function returned by the hook useNextUpload, why does it eat the possible errors ? Is it for security reasons, to not let confidential information leaking to the client ? Then, why not throw a generic error instead ?

In my use case, I'm finding myself with some failed upload requests (due to external reasons) and since the errors are eaten, the app continues the flow and I end up with corrupted orders in my database, which can result in a bad deliver for my clients.

If it's indeed a bug, I can propose a PR. If it's not, let me know your reasons, please.

Thanks

TimMikeladze commented 4 days ago

This is indeed an oversight. PRs welcome.

vegandiet705 commented 4 days ago

@TimMikeladze Actually for my use case, I think it's better to use a server action like the following:

'use server'

import { nup } from '@/app/upload/nup';

async function idExistInBucket(id: string): Promise<boolean> {
    try {
        await nup.init();
        await nup.getAsset({ id });
    }
    catch (error) {
        return false;
    }

    return true;
}

export async function idsExistInBucket(ids: string[]): Promise<Map<string, boolean>> {
    const resultMap = new Map<string, boolean>();
    const promises = ids.map(async id => {
        const exists = await idExistInBucket(id);
        resultMap.set(id, exists);
    });

    await Promise.all(promises);

    return resultMap;
}

With this server action, I can do the existence verification before uploading files in the client, avoiding upload requests for already existent files:

    const uploadFiles = async () => {
        const orderAndFilesArray = await cacheOperator.getOrderAndFilesArray(orderIds);

        await Promise.all(
            orderAndFilesArray.map(async (orderAndFiles) => {
                const files = orderAndFiles.files.map((file) => {
                    return { id: `${file.imageId}`, file: blobToFile(arrayBufferToBlob(file.file, file.type), `${file.imageId}`) };
                });
                const filesIds = files.map(file => file.id);
                const resultMap = await idsExistInBucket(filesIds);
                const uploadPromises = [];

                for (const file of files) {
                    if (!resultMap.get(file.id)) {
                        uploadPromises.push(nup.upload(file));
                    }
                }

                await Promise.all(uploadPromises);
            })
        );
    };

This not only ensures error prevention but also idempotence. So, I'm retreating on my PR suggestion for now. But how do you think it should be done ? My idea was just adding a throw("Generic error") inside the upload's catch block, in the file src/react/useNextUpload.tsx.