appwrite / sdk-for-android

[READ-ONLY] Official Appwrite Android SDK 💚 🤖
https://appwrite.io
BSD 3-Clause "New" or "Revised" License
114 stars 20 forks source link

Exceptions are a serious problem. #41

Closed muratdoglu closed 5 months ago

muratdoglu commented 1 year ago

👟 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?

🏢 Have you read the Code of Conduct?

balachandarlinks commented 11 months ago

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 commented 11 months ago

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 commented 9 months ago

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?

muratdoglu commented 8 months ago

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

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

}

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

getCustomers(
{ },
{ },
{ })
abnegate commented 5 months ago

@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:

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

balachandarlinks commented 5 months ago

@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.