uber / RxDogTag

Automatic tagging of RxJava 2+ originating subscribe points for onError() investigation.
https://uber.github.io/RxDogTag
Apache License 2.0
648 stars 19 forks source link

Add support for onError handler #69

Closed snorkel123 closed 3 years ago

snorkel123 commented 3 years ago

In certain cases like OAuth you do need to set onError handler, so some helper function would be nice.

For now, I created helper function handleOnError that I attach to every onError thus mimicking RxDogTag.

Gist for helper function.

ZacSweers commented 3 years ago

What's the ask?

snorkel123 commented 3 years ago

What's the ask?

Add support for onError handler

ZacSweers commented 3 years ago

Again, what specifically do you propose? "Support onError handler" is vague and unhelpful on its own.

snorkel123 commented 3 years ago

Ok, I will explain more in depth.

In certain cases you need to have onError still set like in OAuth, but do not want to abandon RxDogTag. For now, RxDogTag does not support subscribe with onError set. I ask for anything that will handle scenario with onError set.

ZacSweers commented 3 years ago

Can you offer a simple reproducing test snippet? I don't see why a custom onError handler would affect anything, RxDogTag works via subscribe hooks and knows nothing about custom error hooks.

snorkel123 commented 3 years ago

Probably misunderstanding. I acknowledge that RxDogTag does not know nothing about custom error hooks. So I propose addition to that, maybe some helper function fot those cases.

ZacSweers commented 3 years ago

Again, you have offered no concrete proposal and commented with an initially passive aggressive comment in response to the above. Please respect our code of conduct and keep it respectful. I'm going to close this issue for now, feel free to revisit if you can offer a concrete proposal (snippet, test case, etc) of what you think this library should accomplish. It sounds like what you're asking for is us to make the stack element tagging APIs public for use with any other Throwable, which we're not willing to do because it's very tightly coupled to RxDogTag internals.

snorkel123 commented 3 years ago

I am surprised with passive agressive thing. Complete misunderstanding, maybe due to cultural difference.

No, I am not asking for using internals there (how did you deduce that?)

snorkel123 commented 3 years ago

My initial comment ("close as wont do") was my interpretation of your comment "I don't see why a custom onError handler would affect anything, RxDogTag works via subscribe hooks and knows nothing about custom error hooks." What is offensive here?

snorkel123 commented 3 years ago

Alright, I re-read this thread and it seems that my ask was indeed a bit vague. Sorry for some mess here. As for small argument before, I hope it is cleared.

I will restate as good as possible.

Ask: Have RxDogTag behaviour with onError set.

Example:

Observable
   // ...
   .subscribe( { }, {
      if(it is SomeException) { someAction() } else { handleOnError(it) } // do not give up on subscription point in stacktrace
} )

Supposed handleOnError function:

import android.util.Log
import rxdogtag2.RxDogTag

class RxErrorHandler {
    companion object {
        const val package_id = "com.test.myapplication"

        fun handleOnError(th: Throwable) {
            val stack = Thread.currentThread().stackTrace
                    .filter { it.className.contains(package_id) }
                    .filterNot { it.className.contains("RxErrorHandler") }
                    .joinToString(separator = "\n")

            var message =
                    stack + "\n" +
                            "[[ ↑↑ Inferred subscribe point ↑↑ ]]" + "\n"

            val sourceLines = th.stackTraceToString()
                    .split("\n")
                    .filter { it.contains(package_id) }
                    .joinToString(separator = "\n")

            if (sourceLines.isNotBlank()) {
                message += ("[[ Error source in your code ]]" + "\n" +
                        th.toString() + "\n" +
                        sourceLines) + "\n"
            }

            message += ("[[ ↓↓ Original trace ↓↓ ]]" + "\n" + th.stackTraceToString())

            // Choose preferred log type
            //Log.e("RxJava", message)
            message.split("\n").forEach { Log.e("RxJava", it) }

            Thread.currentThread().uncaughtExceptionHandler
                    ?.uncaughtException(Thread.currentThread(), th)
        }
    }

}

Sample stacktrace this function produces for following snippet:

Observable
            .just(1)
            .subscribeOn(Schedulers.io())
            .map { null }
            .observeOn(AndroidSchedulers.mainThread())
            .subscribe({}, { handleOnError(it) })
com.test.myapplication.MainActivity$onCreate$3.accept(MainActivity.kt:26)
    com.test.myapplication.MainActivity$onCreate$3.accept(MainActivity.kt:14)
    [[ ↑↑ Inferred subscribe point ↑↑ ]]
    [[ Error source in your code ]]
    java.lang.NullPointerException: The mapper function returned a null value.
        at com.test.myapplication.MainActivity.onCreate(MainActivity.kt:24)
        at com.test.myapplication.MainActivity.onCreate(MainActivity.kt:25)
    [[ ↓↓ Original trace ↓↓ ]]
    java.lang.NullPointerException: The mapper function returned a null value.
        at java.util.Objects.requireNonNull(Objects.java:228)
    ...
snorkel123 commented 3 years ago

Any traction here?

ZacSweers commented 3 years ago

I'm going to give you the benefit of the doubt and assume that you are genuinely trying to repair this, but it is important that you understand that blaming anyone else's "culture" for your own behavior is unacceptable, incompatible with our CoC, and any future issues like this will result in a locked thread and closed without comment. Immediately editing your comments does not hide their history. This is your one and only warning.

RxDogTag supports what you are asking for, create your own observer and make it implement RxDogTagErrorReceiver. See its tests for usage examples. We don't generally advise this as the point of RxDogTag is only for cases that do not have custom error handling, hence why it's not described in the README.

snorkel123 commented 3 years ago

Hi Zac,

Thanks for your answer! Now I understand how my idea fits into RxDogTag.

Honestly, my comment (one that you disliked) was triggered by your provocative way of replying before. However, I am intrerested only in making other projects better, hence I edited my comment to continue constructive dialog.

For me, sayings like "This is your one and only warning" make me want to abandon Github altogether. It's very alien to air of Github and I would never tell it to anyone in any situation on Github.

You say I blame culture - this is only your interpretation, and I do not intend to touch it.

Overall, thank you for constructive position too.