sdk-for-android icon indicating copy to clipboard operation
sdk-for-android copied to clipboard

Exceptions are a serious problem.

Open muratdoglu opened this issue 9 months ago • 4 comments

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

  • [X] I checked and didn't find similar issue

🏢 Have you read the Code of Conduct?

muratdoglu avatar Sep 22 '23 08:09 muratdoglu

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.

balachandarlinks avatar Oct 19 '23 11:10 balachandarlinks

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.

oksimple avatar Nov 06 '23 03:11 oksimple

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 avatar Dec 14 '23 10:12 abnegate

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

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

}

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

getCustomers(
{ },
{ },
{ })

muratdoglu avatar Jan 18 '24 20:01 muratdoglu

@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

abnegate avatar Apr 24 '24 09:04 abnegate

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

balachandarlinks avatar May 04 '24 08:05 balachandarlinks