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

Exceptions are a serious problem. #41

Closed
2 tasks done
muratdoglu opened this issue Sep 22, 2023 · 6 comments
Closed
2 tasks done

Exceptions are a serious problem. #41

muratdoglu opened this issue Sep 22, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@muratdoglu
Copy link

👟 Reproduction steps

Sending exceptions in every negative case of the application leads to serious issues. Is it correct to use try-catch everywhere.

Even though I don't use try-catch, some exceptions still cause the application to crash. Firebase Crashlytics is full of crashes. We need to urgently move away from this try-catch approach. If necessary, it should be handled within the SDK. We shouldn't force the person using the SDK to put try-catch everywhere; it's not clean code at all.

👍 Expected behavior

lol

👎 Actual Behavior

lol

🎲 Appwrite version

Appwrite Cloud

💻 Operating system

Linux

🧱 Your Environment

lol

👀 Have you spent some time to check if this issue has been raised before?

  • I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

@muratdoglu muratdoglu added the bug Something isn't working label Sep 22, 2023
@balachandarlinks
Copy link

Totally agree. Exceptions thrown by Realtime is not catchable as well with try-catch. It crashes the app straightaway. Realtime should use a callback for errors instead of throwing exceptions.

@oksimple
Copy link

oksimple commented Nov 6, 2023

The Android client is an important user group of the BAAS platform architecture. It is really confusing to use such a large number of exception handling methods.

@abnegate
Copy link
Member

Hi @muratdoglu, we are always open to improvements, in what way would you prefer to handle errors? Maybe methods that return an error object or similar instead of throwing, or did you have something else in mind?

@abnegate abnegate added enhancement New feature or request and removed bug Something isn't working labels Dec 14, 2023
@abnegate abnegate changed the title 🐛 Bug Report: Exceptions are a serious problem. Exceptions are a serious problem. Dec 14, 2023
@muratdoglu
Copy link
Author

@abnegate I think, It's a good sample;

   fun getUsers ( 
     successHandler: (SuccessResponse) -> Unit,
     failureHandler: (Throwable?) -> Unit,
     errorHandler: (ErrorResponse?) -> Unit
) {

}

-------------

getCustomers(
{ },
{ },
{ })

@abnegate
Copy link
Member

@muratdoglu While that kind of pattern is common for callback based asynchronous code, when using structured concurrency like Kotlin coroutines, it is common to use exceptions. Doing so leads to allowing cleaner and more straight forward code when compared to callbacks.

There are another couple of options, but neither quite provide the same flexibility while sticking to language idioms:

  • Null returns: Easier to handle, but all information about what went wrong is lost.
  • Result<T, Exception> returns: Just requires an if(result.success) block instead of try/catch around the function.

For these reasons, we'll stick with exceptions for now

@balachandarlinks
Copy link

@abnegate Can you provide an example on how we can handle exceptions thrown by realtime? Please refer this example in realtime docs. There is no way to catch the exception here and they result in app crash.

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

4 participants