payloadcms / payload

Payload is the open-source, fullstack Next.js framework, giving you instant backend superpowers. Get a full TypeScript backend and admin panel instantly. Use Payload as a headless CMS or for building powerful applications.
https://payloadcms.com
MIT License
23.37k stars 1.49k forks source link

Can't use focal points or cropping in 2.4.0 with plugin-cloud-storage #4422

Closed jamesvclements closed 4 months ago

jamesvclements commented 9 months ago

Link to reproduction

No response

Describe the Bug

After cropping or setting the focal point and pressing save, I get this error:

FileUploadError: There was a problem while uploading the file.
    at generateFileData (cms/node_modules/payload/src/uploads/generateFileData.ts:81:13)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at updateByID (cms/node_modules/payload/src/collections/operations/updateByID.ts:142:57)
    at updateByIDHandler (cms/node_modules/payload/src/collections/requestHandlers/updateByID.ts:36:17)

Also, when setting focal point on a new upload, the image will save, but the focal point won't be saved. Then trying to edit it, get the same error as the one above.

Here's a video walkthrough: https://share.cleanshot.com/pbC2DvJN

To Reproduce

Here's my cloud storage config:

payloadCloudStorage({
  collections: {
    'media': {
      adapter: s3Adapter({
        config: {
          credentials: {
            accessKeyId: process.env.CLOUDFLARE_ACCESS_KEY_ID,
            secretAccessKey: process.env.CLOUDFLARE_SECRET_ACCESS_KEY,
          },
          region: 'auto',
          endpoint: process.env.CLOUDFLARE_ENDPOINT,
        },
        bucket: process.env.CLOUDFLARE_BUCKET,
      }),
    },
  }
}),

And here's my media collection config:

export const Media: CollectionConfig = {
  slug: 'media',
  access: {
    read: () => true,
  },
  upload: {
    disableLocalStorage: true,
    adminThumbnail: 'thumbnail',
    imageSizes: [
      {
        name: 'webp',
        formatOptions: {
          format: 'webp',
          options: {
            quality: 100
          }
        },
        withoutEnlargement: true,
      },
      {
        name: 'webp_1280',
        width: 1280,
        formatOptions: {
          format: 'webp',
        },
        withoutEnlargement: true,
      },
      // This is used for thumbnails in the CMS
      {
        height: 256,
        width: 256,
        position: 'center',
        name: 'thumbnail',
      },
    ],
  },

Payload Version

2.4.0

Adapters and Plugins

payloadCloudstorage

jamesvclements commented 9 months ago

Possibbly #4336, doesn't seem to be fixed by https://github.com/payloadcms/plugin-cloud-storage/issues/81

JessChowdhury commented 9 months ago

Hi @jamesvclements, okay I think we have 2 separate problems happening:

  1. I believe you're getting the FileUploadError error because plugin-cloud-storage is trying to use an incorrect fileURL.

    In the payload-cloud-storage plugin, the s3 adapter generates the fileURL by combining the provided [endpoint]/[bucket]/[filename]. However, with Cloudflare R2 you need to use the public bucket URL instead of the endpoint.

    This is where to find it in your bucket > settings:

    Screenshot 2023-12-12 at 11 13 16 AM

    Then add it to your plugin like this:

      cloudStorage({
        collections: {
          media: {
            adapter, 
            generateFileURL: ({ filename }) => {
              return `${process.env.CLOUDFLARE_PUBLIC_ENDPOINT}/${process.env.CLOUDFLARE_BUCKET}/${filename}`
            },
        },
      },
      }),

    Give this a try and let me know if it works!

  2. The focal point co-ordinates in the admin panel are somewhat misleading because it resets to center (50, 50) after save and does not retain the previously applied focal point coords. We have plans to improve this in the future, as well as exposing the focal point coords from the API.

    As for the focal point not applying on the initial save - this would definitely be a bug and I will look into this further.

JessChowdhury commented 9 months ago

An alternative solution, instead of passing generateFileURL to the plugin, you should also be able to set your media collection staticURL to ${process.env.CLOUDFLARE_PUBLIC_ENDPOINT}/${process.env.CLOUDFLARE_BUCKET} to achieve the same thing.

JessChowdhury commented 9 months ago

@jamesvclements the focal point bug has been fixed here and will be out in our next release.

Also let me know if you tried adding generateFileURL to the cloudStorage plugin and if that worked or you still need help.

jamesvclements commented 9 months ago

@JessChowdhury Thanks for looking into this! I'm a little confused about the first issue, why would I be having the upload issue only after setting the focal point, and not when uploading images normally? The plugin-cloud-storage works fine as-is, without exposing the bucket publicly or adding generateFileURL, it's only after setting the focal point that the error appears.

jamesvclements commented 9 months ago

@JessChowdhury Adding generateFileURL breaks my frontend which is expecting relative paths like /media/filename.jpg and then prefixes the CMS domain in front of that, e.g. cms.site.com, When I add generateFileURL, the URLs returned by all media items are the absolute URLs from Cloudflare. I could adjust my frontend, but I'm confused now how it's been working fine with the relative paths and letting Payload spit back the media I need (keeping the Cloudflare bucket private).

jamesvclements commented 9 months ago

@JessChowdhury wondering if it has to do with this: https://github.com/payloadcms/plugin-cloud-storage?tab=readme-ov-file#payload-access-control

Edit: Okay yeah this almost certainly has something to do with it. If I add disablePayloadAccessControl, my media items get the full Cloudflare URLs (same effect as the generateFileURL code suggested above), but my frontend breaks because my bucket isn't public.

I don't (think) I should have to make the bucket public though to make this work? With disablePayloadAccessControl set to false (the default), it's able to access the images via the API key and return them. So the cropping / focal point code should be able to do the same...? Then it should be able to access files through the /media/filename.jpg relative URL which uses the Payload access control to get access to the bucket

Just to make this super clear. With disablePayloadAccessControl: true OR the generateFileURL suggested above: Dashboard - Payload-Wednesday-December-13-2023-11 51 25AM@2x Those URLs don't work, they get forbidden access warnings because the bucket isn't public.

Without those (the default): Dashboard - Payload-Wednesday-December-13-2023-11 52 57AM@2x The URLs do work...I guess because the plugin passes it through:

This plugin will "pass through" all files that are hosted on your third-party cloud service—with the added benefit of keeping your existing access control in place.

jamesvclements commented 9 months ago

Dove into the code a little bit, I (think) that it's working with private buckets because if you don't pass generateFileURL it uses the value returned by default from the adapter, and the adapter has access via the API key.

https://github.com/payloadcms/plugin-cloud-storage/blob/c9844f2958f249ae46cddbf5ce2241ce08dff15f/src/hooks/afterRead.ts#L13

JessChowdhury commented 9 months ago

@JessChowdhury Thanks for looking into this! I'm a little confused about the first issue, why would I be having the upload issue only after setting the focal point, and not when uploading images normally? The plugin-cloud-storage works fine as-is, without exposing the bucket publicly or adding generateFileURL, it's only after setting the focal point that the error appears.

It is different the second time (when you change the focal point) because then uploadEdits will be true when it hits this code.

I think the issue might be something simple within this block, I will investigate further and get back to you.

Do you have a serverURL defined in your payload config? I am noticing that this also affects the file urls.

UPDATE:

Even when the URL is correct, the request gets denied access. We need to fix the authentication during the request then this should be working as expected - I'll work on this first thing tomorrow and get back to you.

pooledge commented 8 months ago

I am still getting this without plugin-cloud-storage on 2.5.0 with the following relevant config:

...
upload: {
    staticURL: '/media',
    staticDir: 'media',
    disableLocalStorage: false,
    ...
}

While everything seems just fine locally, this error occurs once deployed with OpenShift using a volume as a persistent storage.

I am setting focal point while re-uploading a media file as a workaround.

carlosfaria94 commented 8 months ago

We are getting the same error when doing some Edit Image operation. We are using plugin-cloud-storage 1.1.1 and payload 2.7.0

cbratschi commented 8 months ago

This is probably related to the still failing fetch: https://github.com/payloadcms/payload/issues/4421#issuecomment-1864867979

This exception is just being ignored and therefore hard to debug or recognize.

ssyberg commented 8 months ago

@JessChowdhury can we reopen this? I'm still having this issue on 2.8.2 even with cloudStorage disabled locally (also broken in production with cloudStorage enabled`

JessChowdhury commented 8 months ago

@ssyberg yes absolutely. I'll take look into it, along with this similar issue.

thijssmudde commented 7 months ago

@ssyberg I have the exact same issue with payload and not being able to readjust focal points after uploading media on payload 2.8.2.

swytch commented 7 months ago

@JessChowdhury - Are you able to check using a config that has staticURL as a full path?

Ie:

export const Media: CollectionConfig = {
  slug: 'media',
  upload: {
    staticURL: `https://public.s3.ap-southeast-2.amazonaws.com/media`,
    ...
  }
}

And also the adapter config :

cloudStorage({
  collections: {
    'media': {
      adapter,
      disablePayloadAccessControl: true,
    }
  },
})

I think the issue stems from getExternalFile not handling full path urls, and always injecting the ${baseUrl}.

If I change the staticURL to /media cropping works, but changing the focal point doesn't. Also, cropping doesn't refresh the image after a successful crop.

ssyberg commented 7 months ago

I'm still a bit unclear on if this is a payload issue or a plugin-cloud-storage issue, should we reopen this in the plugin repo (can't actually remember if it's a separate repo now) @JessChowdhury

DanRibbens commented 7 months ago

Hey @ssyberg! We transfered all issues and the plugin code to the monorepo now so we can work off of this issue as-is. Thanks.

denolfe commented 5 months ago

This was likely fixed with https://github.com/payloadcms/payload/pull/5102.

Please let me know if you're still seeing this issue in latest.

andershermansen commented 5 months ago

@denolfe I'm still getting this issue when setting a focal point. I'm using a Azure Storage and not S3.

Versions:

Payload 2.12.1
@payloadcms/plugin-cloud-storage 1.1.2
@azure/storage-blob 12.17.0

It works fine in local dev environment (npm run dev) also running against Azure Storage Environment.

But after deploying (to Azure Web App) in production mode I get error.

In browser: 400 Bad request Response: {"errors":[{"message":"There was a problem while uploading the file."}]}

In server side:

2024-04-06T08:20:43.522305582Z: [INFO]  [08:20:42] ERROR (payload): FileUploadError: There was a problem while uploading the file.
2024-04-06T08:20:43.522358383Z: [INFO]      at generateFileData (/home/site/wwwroot/node_modules/payload/dist/uploads/generateFileData.js:61:19)
2024-04-06T08:20:43.522365783Z: [INFO]      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
2024-04-06T08:20:43.522370183Z: [INFO]      at async updateByID (/home/site/wwwroot/node_modules/payload/dist/collections/operations/updateByID.js:107:61)
2024-04-06T08:20:43.522374683Z: [INFO]      at async updateByIDHandler (/home/site/wwwroot/node_modules/payload/dist/collections/requestHandlers/updateByID.js:35:21)
andershermansen commented 5 months ago

@denolfe there is no logging of the root cause here. But are there any temp files on the local filesystem here when generering the new image files for new focal point?

if that is the case then maybe that is why its failing for me because the Azure Web App is running from package in read only mode.

denolfe commented 5 months ago

@andershermansen I'd like to determine if this is the same issue or just the same (vague) error. Seems like it has to do specifically with your deployment environment.

The only time files are written to disk is if you have upload options specified on your config

upload: {
  useTempFiles: true
}
andershermansen commented 5 months ago

@denolfe So how can we move forward here to get to the root cause? Any way we could log more about the underlying error instead of just "FileUploadError: There was a problem while uploading the file." when it happens?

denolfe commented 5 months ago

We might need to add better logging of the error. Can you try setting disableLocalStorage to true on your collection?

cbratschi commented 5 months ago

@denolfe I proposed a better logging here:

https://github.com/payloadcms/payload/issues/4421#issuecomment-2064129906

Use the Error cause property to keep tracking the root cause. A lot can fail in those function calls.

PS: please have a look at Next.js OTel implementation, using OTel would make a lot of sense to track errors and to use it for profiling. Especially database calls can be time critical.

andershermansen commented 5 months ago

We might need to add better logging of the error. Can you try setting disableLocalStorage to true on your collection?

@denolfe Same error with disableLocalStorage set to true. This is on payload 2.13.0

2024-04-22T07:57:13.164558962Z: [INFO]  [07:57:12] ERROR (payload): FileUploadError: There was a problem while uploading the file.
2024-04-22T07:57:13.173416594Z: [INFO]      at generateFileData (/home/site/wwwroot/node_modules/payload/dist/uploads/generateFileData.js:62:19)
2024-04-22T07:57:13.173447594Z: [INFO]      at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
2024-04-22T07:57:13.173453294Z: [INFO]      at async updateByID (/home/site/wwwroot/node_modules/payload/dist/collections/operations/updateByID.js:107:61)
2024-04-22T07:57:13.173457594Z: [INFO]      at async updateByIDHandler (/home/site/wwwroot/node_modules/payload/dist/collections/requestHandlers/updateByID.js:41:21)
andershermansen commented 5 months ago

@denolfe I tried to build my project with a local payload to reproduct this with more logging enabled. But was unable to do so. If you have time to look at how I do it I created a request for help in discord. Ref https://discord.com/channels/967097582721572934/1231948495095857234/1231948495095857234

100tomtomas100 commented 4 months ago

andershermansen

I have exact same errors with both croping and focalpoint. My S3 is in backblaze

drosteluk commented 4 months ago

denolfe andershermansen

I was able to fix it by changing uploads/getExternalFile.js line 62/63 from:

` // Convert headers

    const convertedHeaders = headersToObject(req.headers);

`

To:

` // Convert headers

    const convertedHeaders = Object.fromEntries(new Headers(req.headers));

`

The error was thrown because the headersToObject function, which generates the convertedHeaders expects an array, but receives an object. This was in Payload 2.

In the Payload 3 Demo the getExternalFile function also misses the if (!url.startsWith('http')) {...} check and always sets the origin endpoint before the s3 endpoint.

Hope this helps to fix the issue.

denolfe commented 4 months ago

@drosteluk Yes, this was the issue. A fix was pushed yesterday in v2.14.0.

@andershermansen Can you see if this fixes the issue for you? If an error occurs, more detail will be shown as well.

I will look into the issue you mentioned in 3.0 👍

andershermansen commented 4 months ago

@denolfe No more error messages with 2.14.1. But it still does not work as expected. If I open an image and click "Edit image" and set a new "Focal point". I then hit "Apply changes" and then "Save".

I can see the thumbnail change to the expected image (with the correct focal point). Then it switches back to the old / center focal point right away again and shows "Updated successfully". So it's still old thumbnail with wrong focal point.

100tomtomas100 commented 4 months ago

@denolfe

I do not have any errors either but focal point does not work. Crop works but it does not change the image until I hard reload the page. Is there a way after crop to delete image from cache ?

drosteluk commented 4 months ago

denolfe

Thanks for the quick fix, i really appreciate the work you put into Payload.

andershermansen

The cloud storage plugin doesn't invalidate the cloudfront cache. You can bypass this by rather using the origin endpoint instead of the cdn endpoint or sending an CreateInvalidationCommand from the @aws-sdk/client-cloudfront in an afterchange hook. This guy did a pretty neat tutorial for this case: https://www.youtube.com/watch?v=lZAGIy1e3JA&t=150s

To prevent browser cache i just add the updatedAt value to the url as a query parameter like this:

` afterRead: [ async ({doc}) => { doc.url = '${doc.url}?${doc.updatedAt}';

      return doc;
  },
]

`

denolfe commented 4 months ago

@denolfe

I do not have any errors either but focal point does not work. Crop works but it does not change the image until I hard reload the page. Is there a way after crop to delete image from cache ?

Are you certain this is a cache issue? Could it simply be that the component isn't fetching the updated image?

andershermansen commented 4 months ago

@drosteluk Thanks for the cache hints. There is no CDN in my scenario, I have hooked payload directly to azure storage. But it was browser cache. The afterRead hook did the trick.

Since I'm using multiple sizes, I needed a bit different hook. I created it like this:

        afterRead: [
            async ({doc}) => {
                doc.url = `${doc.url}?updated=${encodeURIComponent(doc.updatedAt)}`;
                for (const size in doc.sizes) {
                    if (doc.sizes[size].url) {
                        doc.sizes[size].url = `${doc.sizes[size].url}?updated=${encodeURIComponent(doc.updatedAt)}`;
                    }
                }
                return doc;
            },
        ],

Thanks for fixing this @drosteluk and @denolfe 🎉

100tomtomas100 commented 4 months ago

@drosteluk

Thank you for the solution it works perfectly !

@denolfe After crop the image does not change. In preview sizes it shows uncroped images. If I inspect the image and open the url in a new tab it shows a croped image

Focal point is working I wrongly interpreted it as not working as after save when you go to edit its being shown in the center again.

denolfe commented 4 months ago

@100tomtomas100 Please open a separate issue for this one. While somewhat related, likely a different fix will be needed.

Closing this as complete.

github-actions[bot] commented 1 week ago

This issue has been automatically locked. Please open a new issue if this issue persists with any additional detail.