Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
daryl-cecile opened this issue Nov 11, 2023 · 2 comments · May be fixed by #489
Open

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

daryl-cecile opened this issue Nov 11, 2023 · 2 comments · May be fixed by #489

Comments

@daryl-cecile
Copy link
Contributor

daryl-cecile commented Nov 11, 2023 •
edited

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.
@daryl-cecile daryl-cecile linked a pull request Nov 11, 2023 that will close this issue
Open
@luismeyer
Copy link
Member

luismeyer commented Dec 18, 2023 •
edited

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
Copy link
Contributor Author

daryl-cecile commented Dec 19, 2023 •
edited

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@daryl-cecile @luismeyer