pingdotgg / uploadthing

File uploads for modern web devs
https://uploadthing.com
MIT License
3.87k stars 282 forks source link

RFC: export more granular error classes #771

Closed juliusmarminge closed 3 months ago

juliusmarminge commented 4 months ago

Describe the feature you'd like to request

Right now we convert all possible errors to a single UploadThing class. This is a) tedious to maintain as you need to think of ways to "rephrase" the base error to the more generic UploadThing one and b) hard to check against as a user since there's no good way to differentiate between errors in for example the error formatter, or client side onUploadError.

Sure there are some different code's but similar to http codes they're quite general and meant to map between (like BAD_REQUEST -> HTTP Status 400 for example).

Describe the solution you'd like to see

Should we export more error classes? Right now we have quite many (see https://github.com/pingdotgg/uploadthing/blob/main/packages/shared/src/tagged-errors.ts). This would allow users to do instanceof a certain error and decide whether it should get rethrown or converted to a more generic one if they don't want to expose the information to the client:

import { InvalidFileSizeError } from "uploadthing/errors";

const f = createUploadThing({
  errorFormatter: (error) => {
    // error can be multiple kinds of errors, not just UploadThingError
    if (error instanceof InvalidFileSizeError) {
      // here you could also access `error.max` etc
      return { message: error.reason };
    }
    // fallback to generic error message
    return { message: error.message };
  }
});

It will still be a balance on us maintainers to decide what to put as message and what to put as reason (naming tbd), but at least we could do that on the base error class once and not have to think about converting it later:

export class InvalidFileSizeError extends TaggedError("InvalidFileSize")<{
  reason: string;
  message: string;
  max: number;
  received: number;
}> {
  constructor(received: number, max: number) {
    // Message is a client-safe message that shouldn't expose any details but just what went wrong
    const message = `The file you attempted to upload is not within the accepted bounds.`

    // Reason (naming tbd) has more details
    const reason = `...`
    // Also forward the raw data (max, received, (maybe also type?))
    super({ reason, message, max, received });
  }
}

[!NOTE] Maybe not all errors would even need to obfuscate stuff, as for example with the file size, that's already exposed on client (or by simply GET /api/uploadthing anyways, where for example FetchErrors to external parties are more sensitive

Additional information

No response

👨‍👧‍👦 Contributing

github-actions[bot] commented 3 months ago

This issue has been automatically marked as stale because it has not had any activity for 10 days. It will be closed in 5 days if no further activity occurs.

github-actions[bot] commented 3 months ago

This issue has been closed because it has not had any activity for 5 days since being marked as stale.