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

Add support for Kotlin's Result #4018

Draft
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Dec 21, 2023

@Goooler
Copy link
Contributor Author

Goooler commented Dec 21, 2023

And if we should include this in default adapters?

@connyduck
Copy link

connyduck commented Mar 1, 2024

I tested this by including a build from this branch in Tusky. A release build crashes immediately with

java.lang.IllegalArgumentException: Unable to create call adapter for class F5.f
	for method d.W0
	at S7.i0.j(SourceFile:40)
	at S7.A.b(SourceFile:2844)
	at S7.c0.c(SourceFile:30)
	at S7.a0.invoke(SourceFile:36)
	at java.lang.reflect.Proxy.invoke(Proxy.java:1006)
	at $Proxy3.W0(Unknown Source)
	at W3.P.t(SourceFile:40)
	at L5.a.i(SourceFile:6)
	at f6.a.c(SourceFile:140)
	at h2.f.R0(SourceFile:12)
	at a6.a.i0(SourceFile:76)
	at R4.l0.D0(SourceFile:30)
	at com.keylesspalace.tusky.MainActivity.onCreate(SourceFile:664)
	at android.app.Activity.performCreate(Activity.java:8051)
	at android.app.Activity.performCreate(Activity.java:8031)
	at android.app.Instrumentation.callActivityOnCreate(Instrumentation.java:1329)
	at android.app.ActivityThread.performLaunchActivity(ActivityThread.java:3608)
	at android.app.ActivityThread.handleLaunchActivity(ActivityThread.java:3792)
	at android.app.servertransaction.LaunchActivityItem.execute(LaunchActivityItem.java:103)
	at android.app.servertransaction.TransactionExecutor.executeCallbacks(TransactionExecutor.java:135)
	at android.app.servertransaction.TransactionExecutor.execute(TransactionExecutor.java:95)
	at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2210)
	at android.os.Handler.dispatchMessage(Handler.java:106)
	at android.os.Looper.loopOnce(Looper.java:201)
	at android.os.Looper.loop(Looper.java:288)
	at android.app.ActivityThread.main(ActivityThread.java:7839)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
	Suppressed: f6.g: [s0{Cancelling}@4de4607, Dispatchers.Main.immediate]
Caused by: java.lang.IllegalStateException: Result must have a generic type (e.g., Result<T>)
	at S7.Z.a(SourceFile:40)
	at S7.c0.a(SourceFile:33)
	at S7.A.b(SourceFile:2686)
	... 27 more

Debug build seems fine.

@Goooler
Copy link
Contributor Author

Goooler commented Mar 1, 2024

Ooops, need to keep kotlin.Result, fixed in 6e9b4b6.

@connyduck
Copy link

connyduck commented Mar 1, 2024

Now it does no longer crash at startup, but I can reproduce this bug that caused me to deprecate my calladapter.

Deobfuscated Stacktrace:

java.lang.ClassCastException: kotlin.Result cannot be cast to com.keylesspalace.tusky.entity.AppCredentials
	at com.keylesspalace.tusky.components.login.LoginActivity$onLoginClick$1.invokeSuspend(LoginActivity.kt:177)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.internal.DispatchedContinuationKt.resumeCancellableWith(DispatchedContinuation.kt:363)
	at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:26)
	at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable$default(Cancellable.kt:21)
	at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:88)
	at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:123)
	at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch(Builders.common.kt:52)
	at kotlinx.coroutines.BuildersKt.launch(Builders.kt:1)
	at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch$default(Builders.common.kt:43)
	at kotlinx.coroutines.BuildersKt.launch$default(Builders.kt:1)
	at com.keylesspalace.tusky.components.login.LoginActivity.onLoginClick(LoginActivity.kt:170)
	at com.keylesspalace.tusky.components.login.LoginActivity.onCreate$lambda$1(LoginActivity.kt:106)
	at android.view.View.performClick(View.java:7441)
	at com.google.android.material.button.MaterialButton.performClick(MaterialButton.java:1218)
	at android.view.View.performClickInternal(View.java:7418)
	at android.view.View.access$3700(View.java:835)
	at android.view.View$PerformClick.run(View.java:28676)
	at android.os.Handler.handleCallback(Handler.java:938)
	at android.os.Handler.dispatchMessage(Handler.java:99)
	at android.os.Looper.loopOnce(Looper.java:201)
	at android.os.Looper.loop(Looper.java:288)
	at android.app.ActivityThread.main(ActivityThread.java:7839)
	at java.lang.reflect.Method.invoke(Native Method)
	at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
	at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
	Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [s0{Cancelling}@b75f08b, Dispatchers.Main.immediate]

To reproduce, go offline and trigger some network calls (sometimes needs a few tries).

I think the main problem is that Result is a value class and as such is partially inlined at runtime.

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.

Add support for Kotlin's Result
2 participants