Error handling in collection hooks.

[richardvanbergen]

I'm trying to upload to a DigitalOcean spaces instance and I'm implementing the recommended hooks as specified in the documentation.

https://payloadcms.com/docs/production/deployment

I've implemented an upload hook (see attached) but I've got two comments.

1. Is there any way to inform the use if something goes wrong?
2. There may be a bug in regards to duplicate filenames, or possibly just a bad implementation on my part. The beforeChange hook doesn't pass the filename that it's going to be saved as. For instance if I upload image.jpg and that filename already exists twice then Payload will save it down as image-2.jpg but only after the beforeChange hook has executed, and data.filename will be image-1.jpg. So it gets saved down in the storage with that name as the key.

const uploadHook: BeforeChangeHook = async ({ data, req }) => {
  try {
    const s3 = new AWS.S3({
      endpoint: (new AWS.Endpoint(process.env.SPACES_REGION)),
      accessKeyId: process.env.SPACES_KEY,
      secretAccessKey: process.env.SPACES_SECRET
    })

    const response = await s3.putObject({
      Bucket: process.env.SPACES_NAME,
      Key: String(data.filename),
      Body: req.file,
      ACL: "private"
    }).promise()

    console.log(response)

    return data
  } catch (e) {
    console.error(e)

    return {}
  }
}

[richardvanbergen]

Also, could we re-export the collection hooks from payload/types?

[jmikrut]

You mean the collection hook types? Absolutely. On it!

[jmikrut]

This has been merged to master and is awaiting release in the next version (0.4.6). Should be deployed later today.

[richardvanbergen]

Sorry one more question/bug. I'm altering the filename as suggested. But the URL that's being returned is prepended with /media/images. For example: /media/images/https://brightvision.fra1.digitaloceanspaces.com.cdn.digitaloceanspaces.com/contact-image-10-8.jpg

const readHook: AfterReadHook = async ({ doc }) => {
  try {
    console.log(doc)
    return {
      ...doc,
      filename: `https://${process.env.SPACES_NAME}.${process.env.SPACES_REGION}.cdn.digitaloceanspaces.com/${doc.filename}`
    }
  } catch (e) {
    return doc;
  }
}

[jmikrut]

Now this one has stumped me. You are altering filename as suggested? Can you show the code that you are using for that? There might be a bug in that implementation. Might need a bit more info on this one!

[richardvanbergen]

Sorry I'm confused. Aren't I supposed to be altering and returning the filename using the AfterReadHook as I posted above? That seems to be what the documentation is suggesting.

Should I be altering it elsewhere?

[jmikrut]

I have no idea why, but I totally did not realize you were providing the hook code. You can alter filename through hooks, and we do mention that in the docs. If you did that though, you would want to make sure to "un-modify" it through a beforeChange hook or similar, because the filename will have been "read-in" as your modified value, and you may end up with some unintended consequences.

To clarify, every time a document is read, even before it's updated, the hooks are run. So an afterRead hook should be careful to not create a modification "loop" or similar.

I would say that personally I would add a different field to your Collection, maybe one that doesn't ever have a value saved to the database and is more considered a "virtual". Say...:

{
  name: 's3URL',
  type: 'text',
  admin: {
    readOnly: true,
  },
  hooks: {
    beforeChange: [
      () => null,
    ],
    afterRead: [
      ({ data }) => `https://${process.env.SPACES_NAME}.${process.env.SPACES_REGION}.cdn.digitaloceanspaces.com/${data.filename}`
    ]
  }
}

That way, this field is never really saved in your database, and you can treat it like a "virtual".

What do you think? PS, I'll update the docs to be more descriptive as they are currently only half-accurate.

[jmikrut]

Hey @richardvanbergen I love it. I'd be pumped to add this as a tutorial / guide for Payload. We have had this on our list to write about but haven't gotten to it yet.

Couple answers:

Is there any way to inform the use if something goes wrong?

Yes, just throw an error and the error's message will bubble up through the API(s). You should be able to then see it in a response. Give it a shot! We have APIError exposed within payload/errors which can be leveraged if you want for status codes, etc. but you could also just throw any error you want.

There may be a bug in regards to duplicate filenames, or possibly just a bad implementation on my part. The beforeChange hook doesn't pass the filename that it's going to be saved as. For instance if I upload image.jpg and that filename already exists twice then Payload will save it down as image-2.jpg but only after the beforeChange hook has executed, and data.filename will be image-1.jpg. So it gets saved down in the storage with that name as the key.

You're right - the filename gets generated in our create operation after the beforeChange hook is fired, but it doesn't have to be this way. We should be able to modify accordingly 👍 pretty quickly actually. Would this benefit you?

[richardvanbergen]

One more question, can I alter the data that's going to be saved in the beforeChange hook? Like adding an extra field? I was thinking about updating the file on S3 based on the MD5 hash, then all the filename problems go away.

[jmikrut]

We'll get a new version published that offers up the filename into beforeChange hooks exactly how it will be saved to the database. Will get to that in the next few days here. 👍

One more question, can I alter the data that's going to be saved in the beforeChange hook? Like adding an extra field? I was thinking about updating the file on S3 based on the MD5 hash, then all the filename problems go away.

Yes you absolutely can modify the data in the beforeChange hook. You may be thinking about the same thing I just commented elsewhere on this Discussion:

#98 (reply in thread)

You could create a "virtual" field with field hooks and have it be nice and tidy. Is this what you mean?

[richardvanbergen]

Yes! The virtual field works quite nicely! I'll be looking forward to the beforeChange change too.

I've just had one more thought, what about the admin thumbnails? If the storage is ephemeral you could end up in a situation with the frontend being fine but the admin breaking. Because as far as I can tell, there's no way to specify the thumbnail source?

[jmikrut]

Great call.

Just merged a PR that extends the upload.adminThumbnail property by, in addition to only allowing it to specify a size, it now also allows a function that returns a full URL to use as a thumbnail. Updated the docs as well with these changes. You can use this function to set an adminThumbnail however you'd like.

#104

Keep these thoughts coming! Really appreciate the feedback.

[DanRibbens]

I opened a new discussion specific to integrating file hosting with Payload uploads. @richardvanbergen if you have an answer for what worked in your case that would be awesome to share. 😃
#220