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

In cases of non 20x responses, return an error from Execute() #11

Merged
merged 3 commits into from
Jul 26, 2021

Conversation

intabulas
Copy link
Contributor

What kind of change does this PR introduce?

This PR allows Execute() to actually return an error if a non 20x response is received from supabase.

What is the current behavior?

Currently, if an error is received, Execute() return a nil error and the response from supabase. This then requires the user to determine if an error occurred by the contents of the response bytes

ex:

resp, err := client.From("sometable").Insert(document, false, "", "", "").Execute()
// error will always be nil with current behavior, if (for example) sometable doesnt exist
if err != nil {
  // nothing to handle
}

What is the new behavior?

resp, err := client.From("sometable").Insert(document, false, "", "", "").Execute()
// error will not be nil if any non 20x error is returned from supabase
if err != nil {
  // print, or log, or return etc
  fmt.Printf("ERROR: %s\n", err)
}

Additional context

I made two decisions that could be changed. The first was to include Hint and Details in the response struct, even though they are not used. The second was do do >= 300 vs 400 on the error check,

@intabulas intabulas changed the title in cases of non 20 responses, actually return an errpr from executeHe… In cases of non 20x responses, return an error from Execute() Jul 18, 2021
@intabulas intabulas mentioned this pull request Jul 18, 2021
@yusufpapurcu
Copy link
Member

I'm on vacation, I'll test it when I'm available.
Thanks for support 👍

@intabulas
Copy link
Contributor Author

sorry about the force push, was working on #12 and realized PR was still open

Copy link
Member

@yusufpapurcu yusufpapurcu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for great work 💯 .
Your code is nice but can you look requested changes?

execute.go Outdated Show resolved Hide resolved
execute.go Outdated Show resolved Hide resolved
execute.go Outdated Show resolved Hide resolved
execute.go Outdated Show resolved Hide resolved
@yusufpapurcu yusufpapurcu merged commit 628bf26 into supabase-community:main Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants