3 ms·

My issue with this is that you're using exceptions for control flow. A user not being valid is expected (duplicate username). A password not matching a regex is

My issue with this is that you're using exceptions for control flow. A user not being valid is expected (duplicate username). A password not matching a regex is also expected. Then, in general (not seen here as there are no types), I like to use a lot of types in my code. The incoming user would be of type UnvalidatedUser, whereas the return type of this function would be StoredUser or something like that to distinguish the incoming user type with the outgoing. I like to attach semantics to a type, not to conditions: https://existentialtype.wordpress.com/2011/03/15/boolean-blindness/ https://existentialtype.wordpress.com/2011/03/15/boolean-bli...

johnfn 2y ago
That's a great point, I didn't even think of that. I would use error types as well, yes.
rerdavies 2y ago
My issue with that is that absolutely NOTHING will ever convince me that returning error codes is a better idea than throwing exceptions. And that you seem to be using 'expected' in some weird cargo-culty sense of the word. An invalid user name is an error, not an expected case.
- nip 2y ago
  I’ll try: Returning a “Result” with union discrimination on the “success” is far superior to throwing in Typescript in my experience { success: false, reason: “user_already_exists” } | { success: true, data: { user_id: string } - By design you are forced to “deal” with the sad path: in order to get to the data, you must handle the error state - Throwing is not type-safe: you cannot know at build time whether you are handling all thrown errors
- yellowapple 2y ago
  > An invalid user name is an error, not an expected case. If you ain't expecting users to input bogus data, then you're putting way too much trust in said users. Put simply: is it a bug in your own code if a user tries to use an invalid username? If yes, then throw an exception. If no, then return an error code. Exceptions represent programmer error; error codes represent user error.
  - swatcoder 2y ago
    > Exceptions represent programmer error; error codes represent user error. No, they represent whatever suits the system design and the house style. They're just tools to pass data and manipulate control flow. If you become dogmatic and start insisting on what they must absolutely represent, you're only going to find yourself compromising design coherence and futily railing at colleagues when different representations are found to be appropriate. With exceptions, it often makes sense to use them when failures inevitably just propogate up through a stack of nested calls or may appear anywhere in a sequence of calls. In these cases, return codes can grossly interfere with legibility and can lead to confusion when interim handlers squelch errors that really should have propogates up directly. These are the kind of choices that are completely reasonable to make within a specific module (like an auth and accounts module), and you can always return to some other default policy at the interface of that module if your project/house-style needs such. Engineering shouldn't feel so dogmatic as you suggest. It's a strong smell when it does.
aleksiy123 2y ago
I would add one suggestion/comment. Use a known set of standard error codes and not a unique error code/error type for each new situation. Error codes are there to hint to clients/callers what action they should potentially take (retry?, backoff) Don't make callers handle 100s of different potential error codes/types. If the whole internet can work with 70 your app can work with less. All of google uses less than 20 https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto https://github.com/googleapis/googleapis/blob/master/google/... Put more specific information in the error message or a secondary status code.
jansommer 2y ago
It's a good point, especially because different return types is well supported in TS. I made the same in plpgsql recently and opted for returning an implicit union of UserSession | Error, by returning UserSession in the signature and raising errors in the function. The alternative was to return json where you'd have to look at the body of the function to figure out what it returns (when successful), as opposed to the signature. I'm not sure if I'm striking the right balance. Yes, the signature is "self-documenting" - until you hit an error!

johnfn 2y ago

That's a great point, I didn't even think of that. I would use error types as well, yes.

rerdavies 2y ago

My issue with that is that absolutely NOTHING will ever convince me that returning error codes is a better idea than throwing exceptions. And that you seem to be using 'expected' in some weird cargo-culty sense of the word. An invalid user name is an error, not an expected case.

nip 2y ago
I’ll try: Returning a “Result” with union discrimination on the “success” is far superior to throwing in Typescript in my experience { success: false, reason: “user_already_exists” } | { success: true, data: { user_id: string } - By design you are forced to “deal” with the sad path: in order to get to the data, you must handle the error state - Throwing is not type-safe: you cannot know at build time whether you are handling all thrown errors
yellowapple 2y ago
> An invalid user name is an error, not an expected case. If you ain't expecting users to input bogus data, then you're putting way too much trust in said users. Put simply: is it a bug in your own code if a user tries to use an invalid username? If yes, then throw an exception. If no, then return an error code. Exceptions represent programmer error; error codes represent user error.
- swatcoder 2y ago
  > Exceptions represent programmer error; error codes represent user error. No, they represent whatever suits the system design and the house style. They're just tools to pass data and manipulate control flow. If you become dogmatic and start insisting on what they must absolutely represent, you're only going to find yourself compromising design coherence and futily railing at colleagues when different representations are found to be appropriate. With exceptions, it often makes sense to use them when failures inevitably just propogate up through a stack of nested calls or may appear anywhere in a sequence of calls. In these cases, return codes can grossly interfere with legibility and can lead to confusion when interim handlers squelch errors that really should have propogates up directly. These are the kind of choices that are completely reasonable to make within a specific module (like an auth and accounts module), and you can always return to some other default policy at the interface of that module if your project/house-style needs such. Engineering shouldn't feel so dogmatic as you suggest. It's a strong smell when it does.

aleksiy123 2y ago

I would add one suggestion/comment. Use a known set of standard error codes and not a unique error code/error type for each new situation. Error codes are there to hint to clients/callers what action they should potentially take (retry?, backoff) Don't make callers handle 100s of different potential error codes/types. If the whole internet can work with 70 your app can work with less. All of google uses less than 20 https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto https://github.com/googleapis/googleapis/blob/master/google/... Put more specific information in the error message or a secondary status code.

jansommer 2y ago

It's a good point, especially because different return types is well supported in TS. I made the same in plpgsql recently and opted for returning an implicit union of UserSession | Error, by returning UserSession in the signature and raising errors in the function. The alternative was to return json where you'd have to look at the body of the function to figure out what it returns (when successful), as opposed to the signature. I'm not sure if I'm striking the right balance. Yes, the signature is "self-documenting" - until you hit an error!