Return a more useful error when authType is omitted #85
Conversation
The new `authType` parameter is necessary to distinguish between the two Config types, with `makeBlobServiceClient` having different paths through a switch depending on its value to narrow the type between the two possible values. When upgrading from 3.3.0 this value won't be set, so users must explicitly set it to `'default'` to maintain the current behaviour, however the default handler throws an error with the config object, which results in the user-facing error `[2024-02-06 17:36:10.217] error: [object Object]`. This change replaces throwing the config with throwing a human-readable string explaining the issue.
MatthewWilkes
force-pushed
the
better-authtype-error
branch
from
6b9b3f3 to
0588991
Compare
|
Thanks for 3.4.0, by the way :) |
| @@ -85,8 +85,7 @@ function makeBlobServiceClient(config: Config) { | |||
| return new BlobServiceClient(serviceBaseURL, new DefaultAzureCredential()); | |||
| } | |||
| default: { | |||
| const exhaustiveCheck: never = config; | |||
| throw new Error(exhaustiveCheck); | |||
| throw new Error('config.authType value is not supported'); | |||
There was a problem hiding this comment.
You need the never check for Typescript so that it's exhaustive. That said, there's probably some way to make more humane. See here: https://medium.com/technogise/type-safe-and-exhaustive-switch-statements-aka-pattern-matching-in-typescript-e3febd433a7a
There was a problem hiding this comment.
@jakeFeldman Would you be open to something like zod for IO type validation? https://www.npmjs.com/package/zod
There was a problem hiding this comment.
I'd prefer to keep the bundle size as small as possible. Looks like zod is about 13.2 kb when gzipped. Happy to explore adding this if it will help prevent this issue for users
There was a problem hiding this comment.
@jakeFeldman Zod would just help to declare a schema for incoming variables and would be able to emit some useful errors.
If there is a way to gain more type safety on incoming variables in the way the plugin is provided to strapi, that would be better, but out of my expertise.
There was a problem hiding this comment.
IMHO, merging this PR would be useful and could hurt noone.
Today, we've encountered this shady error after upgrading from 3.1.3 to 3.4.0.
It would be nice to see this human-readable error message ❤️
There was a problem hiding this comment.
@TamasHodi It would harm the ability of the code to compile.
The author just needs to retain the never and enrich with a message.
There was a problem hiding this comment.
@TamasHodi yes the package could handle this better. @fardarter I'd be okay moving to zod if someone wants to take on the changes. If not, happy to update just to using the never type as well. Sorry I don't have the bandwidth to take this on right now but will review and publish any changes. Thanks again
The new
authTypeparameter is necessary to distinguish between the two Config types, withmakeBlobServiceClienthaving different paths through a switch depending on its value to narrow the type between the two possible values. When upgrading from 3.3.0 this value won't be set, so users must explicitly set it to'default'to maintain the current behaviour, however the default handler throws an error with the config object, which results in the user-facing error[2024-02-06 17:36:10.217] error: [object Object].This change replaces throwing the config with throwing a human-readable string explaining the issue.