Error handling in collection hooks.

richardvanbergen

2 years ago

1 3

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 {}
  }
}

• Selected Answer
  

  jmikrut

  Payload Team

  2 years ago

  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?

  6 replies
• 

  richardvanbergen

  2 years ago

  Yes, just throw an error and the error's message will bubble up through the API(s).

  So is it OK to just allow the S3 errors to bubble up then? Cool!

  We should be able to modify accordingly 👍 pretty quickly actually. Would this benefit you?

  Unless there's a use case I'm not thinking about - I think it makes more sense.

  It should be how it's going to be saved onto the file system. As it stands the only accurate way for me to get the filename is use the AfterChangeHook which means that if there's an error in the S3 upload then it still gets saved to the database, meaning we might not be able to fetch it later.

  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.

  No problem. I was actually thinking about tidying it up, adding some tests and docs and throwing it up on npm. You're also welcome to steal any code you like from me in the meantime. (Though, you might want to fix it before you use it 😄 )

  Here is my prototype: https://gist.github.com/richardvanbergen/a21c701632a2f24a013d8e17495f3e2c

  Used like so:

  const s3Hooks = withS3Storage(
    {
      endpoint: (new AWS.Endpoint(`${process.env.SPACES_REGION}.digitaloceanspaces.com`)),
      accessKeyId: process.env.SPACES_KEY,
      secretAccessKey: process.env.SPACES_SECRET,
    },
    {
      bucket: process.env.SPACES_NAME
    },
    {
      beforeValidate: [
        // ...
      ]
    } // <- combine with other hooks here
  )
  

• 

  richardvanbergen

  2 years ago

  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

  Payload Team

  2 years ago

  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

  2 years ago

  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

  Payload Team

  2 years ago

  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

  Payload Team

  last year

  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

• 

  richardvanbergen

  2 years ago

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

  2 replies
  

  jmikrut

  Payload Team

  2 years ago

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

  

  jmikrut

  Payload Team

  2 years ago

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

• 

  richardvanbergen

  2 years ago

  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;
    }
  }
  

  3 replies
  

  jmikrut

  Payload Team

  2 years ago

  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

  2 years ago

  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

  Payload Team

  2 years ago

  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.