vercel / storage

Vercel Postgres, KV, Blob, and Edge Config
https://vercel.com/storage
Apache License 2.0
506 stars 56 forks source link

@vercel/blob - Return more useful error message when retrieveClientToken throws #488

Open daryl-cecile opened 11 months ago

daryl-cecile commented 11 months ago

As a user I should be able to know when retrieveClientToken failed for a specific reason. See the following example:

const jsonResponse = await handleUpload({
    body,
    request,
    onBeforeGenerateToken: async ( pathname: string, /* clientPayload?: string, */ ) => {
      // Generate a client token for the browser to upload the file

      // ⚠️ Authenticate users before generating the token.
      // Otherwise, you're allowing anonymous uploads.
      const profile = await getCurrentProfile();

      if (!profile) {
        console.log('User is not logged in');
        throw new Error('Not authenticated');
      }

      if (!userAccessControl.canUser(profile).createOwn(Resources.UPLOADS).granted) {
        console.log('User does not have permission to upload files');
        throw new Error('You do not have permission to upload files');
      }

      return {
        allowedContentTypes: ['application/pdf', 'text/plain'],
        maximumSizeInBytes: 8_000_000,
        tokenPayload: JSON.stringify({ profileId: profile.id })
      };
    },
    onUploadCompleted: async ({ blob, tokenPayload }) => {
      // ...
    }
});

Assuming the above is set up in a route to handle uploads, I should receive 'You do not have permission to upload files' when the user is authenticated but not permitted to perform uploads, instead of the current 'Failed to retrieve the client token' error.

It seems the library is already set up to return the error in the body to the client, so we could attempt to parse the body for the error message.

luismeyer commented 9 months ago

hey @daryl-cecile,

I assume you want to display the different error messages to your users right? While I recognize the issue you're encountering, the approach suggested in your PR might not be ideal. It introduces a requirement that the route handler must include an error field in their response. Although this is a method demonstrated in our documentation, relying on such an implicit convention is not advisable for our SDK. We don't have control over the responses returned by the route handler.

Currently, the best solution I can think of is, exposing the response object somehow to the client. Alternatively, you might consider verifying the user's access privileges before permitting the upload.

Given that a month has passed since you opened this issue, have you developed a solution that effectively addresses your needs?

daryl-cecile commented 9 months ago

Hi @luismeyer this makes sense. Exposing the response might make for a better agnostic approach.

Another use-case I found is trying to limit user upload (e.g. for their given tier, user has allocated storage, and if they go above their allocation, to return an error letting them know)

Right now to cover my use cases, i have branched off (the same changes in the PR) and have linked the project dependency to that branch instead of the latest SDK.

I'll have a play about and see how to expose the response. Though this might need a bigger change