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.

Sign up for GitHub

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

Jump to bottom

Add support for Kotlin's Result #4018

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

Add support for Kotlin's Result #4018

wants to merge 9 commits into from
+217 −4

Conversation

Goooler
Copy link
Contributor

@Goooler Goooler commented Dec 21, 2023 •
edited
Loading

@Goooler
Copy link
Contributor Author

Goooler commented Dec 21, 2023

And if we should include this in default adapters?

@Goooler Goooler mentioned this pull request Mar 1, 2024
Closed
@connyduck
Copy link

connyduck commented Mar 1, 2024 •
edited
Loading

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 •
edited
Loading

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.

@Goooler Goooler mentioned this pull request Mar 1, 2024
Draft
@Goooler
Copy link
Contributor Author

Goooler commented Mar 1, 2024 •
edited
Loading

Reproduced! You are right, this is a bug of Kotlin, see:

@Goooler Goooler marked this pull request as draft March 3, 2024 14:17
@michaelbull michaelbull mentioned this pull request Mar 18, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add support for Kotlin's Result
2 participants
@Goooler @connyduck