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.

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

Strongly Typed Errors #405

Closed
malobre opened this issue Feb 3, 2021 · 5 comments
Closed

Strongly Typed Errors #405

malobre opened this issue Feb 3, 2021 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@malobre
Copy link

malobre commented Feb 3, 2021

Feature request

Is your feature request related to a problem? Please describe.

It is currently impossible to differentiate errors, e.g: when a user signs up there is a multitude of possible errors (network errors, database errors, etc) and although you can easily check if an error occured (if (res.error !== null) { /**/ }) it is not possible to know which error happened.

Describe the solution you'd like

Enum error types:

enum SignUpError {
  InvalidEmail,
  SignUpsDisabled,
  NetworkError,
  /* etc */
}

enum SignInError {
  InvalidCredentials,
  /* etc */
}
export default class GoTrueClient {
  // --snip--
  signUp(credentials: {
      email: string;
      password: string;
  }): Promise<{ /* success object */ } | SignUpError>;
  
  signIn(credentials: {
      email?: string;
      password?: string;
      provider?: Provider;
  }): Promise<{ /* success object */ } | SignInError>;
  // --snip--
}

Describe alternatives you've considered

I am not aware of any alternative.

@malobre malobre added the enhancement New feature or request label Feb 3, 2021
@malobre
Copy link
Author

malobre commented Feb 3, 2021

This would be a BIG DX improvement. I am willing to take a stab a this.

@kiwicopple
Copy link
Member

kiwicopple commented Feb 4, 2021

we also have this open here: supabase/supabase#12739

We have a few options:

  1. Change the error format on the API gateway, but then each
  2. Change the error format in each of the libraries (gotrue-js, postgrest-js, realtime-js)
  3. Change the error format in this wrapper library (supabase-js)

I'm thinking that 2 might be the best option. It won't solve the error format for anyone doing cURL requests, but each of the tools has their opinion, and then our libraries are the place where we have been adding (small) DX improvements - changing defaults etc.

@malobre
Copy link
Author

malobre commented Feb 4, 2021

Oops, I completely missed supabase/supabase#12739.
I agree that 2 is the best option, there is a real need for harmonization across the three underlying librairies.
Being able to handle different errors separately is the only hurdle I currently have with supabase.

@pixtron
Copy link
Contributor

pixtron commented Jul 31, 2022

Yet another aspect to consider for DX is error handling. With typescript errors in catch are either of type unknown or any (depending on useUnknownInCatchVariables). We all know that any isn't a great choice as it renders TS kind of useless. So we set useUnknownInCatchVariables and TS now forces us to drilldown potential errors with type guards.

1. Errors as classes

class SupabaseError extends Error {
  constructor(
    public override message: string, 
    public code: string
  ) {
      super(message);
  }
}

const dispatchRequest = (): void => {
  throw new SupabaseError('The request timed out', 'NETWORK_FAILURE');
}

Handle the error:

try {
  dispatchRequest();
} catch(err: unknown) {
  if (err instanceof SupabaseError) {
    switch (err.code) {
      case 'NETWORK_FAILURE':
        // handle network errors
      break;
    }
  }
  // re throw unhandled error
  throw err;
}

2. Errors as objects

interface SupabaseError {
  code: string
  message: string
}

const dispatchRequest = (): void => {
  throw { code: 'NETWORK_FAILURE', message: 'The request timed out' } as SupabaseError;
}

Handle the error:

try {
  dispatchRequest();
} catch(err: unknown) {
  if (err.code === 'NETWORK_FAILURE') // object is of type unkown
  if (typeof err === 'object' && err.code === 'NETWORK_FAILURE') // object is possibly null
  if (typeof err === 'object' && err !== null && err.code === 'NETWORK_FAILURE') // property code does not exist on type object
  if (typeof err === 'object' && err !== null && 'code' in err && err.code === 'NETWORK_FAILURE') // still: property code does not exist on type object
}

Custom Typeguard to the rescue:

const isSupabaseError = (target: unknown): target is SupabaseError => {
  return (
    typeof target === 'object' 
      && target !== null 
      && 'code' in target 
      && 'message' in target
  );
}

try {
  dispatchRequest();
} catch(err: unknown) {
  if (isSupabaseError(err)) {
    switch (err.code) {
      case 'NETWORK_FAILURE':
        // handle network errors
      break;
    }
  }
  // re throw unhandled error
  throw err;
}

Closely looking at isSupabaseError at runtime can we really be sure that an target is a SupabaseError? No, this typeguard just assures that it is an object with the properties code and message, but it might have any other random property. Whereas with a class and isntanceof we can be 100% sure the error is a SupabaseError.

@monicakh monicakh transferred this issue from supabase/supabase-js Aug 18, 2022
@kangmingtay
Copy link
Member

Hey everyone, we've taken this feedback into consideration and this is now available in the rc branch (soon to be released in supabase-js/v2)! Will be closing this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants