MainActivity: proper alerts for each error type

Also moved the PageViewModel into its own file? What's the norm?

app/src/main/java/dev/lowrespalmtree/comet/MainActivity.kt

@@ -11,18 +11,10 @@ import android.view.inputmethod.EditorInfo
 import android.view.inputmethod.InputMethodManager
 import androidx.appcompat.app.AlertDialog
 import androidx.appcompat.app.AppCompatActivity
-import androidx.lifecycle.MutableLiveData
-import androidx.lifecycle.ViewModel
 import androidx.lifecycle.ViewModelProvider
-import androidx.lifecycle.viewModelScope
 import androidx.recyclerview.widget.LinearLayoutManager
 import dev.lowrespalmtree.comet.databinding.ActivityMainBinding
-import kotlinx.coroutines.*
+import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.channels.onSuccess
-import java.net.ConnectException
-import java.net.SocketTimeoutException
-import java.net.UnknownHostException
-import java.nio.charset.Charset
 
 @ExperimentalCoroutinesApi
 class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen {
@@ -129,16 +121,23 @@ class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen {
                     currentUrl = event.uri
                     visitedUrls.add(event.uri)
                 }
-                is PageViewModel.RedirectEvent -> openUrl(event.uri, redirections = event.redirects)
+                is PageViewModel.RedirectEvent -> {
-                is PageViewModel.FailureEvent -> alert(event.message)
+                    openUrl(event.uri, base = currentUrl, redirections = event.redirects)
+                }
+                is PageViewModel.FailureEvent -> {
+                    var message = event.details
+                    if (!event.serverDetails.isNullOrEmpty())
+                        message += "\n\n" + "Server details: ${event.serverDetails}"
+                    alert(message, title = event.short)
+                }
             }
             event.handled = true
         }
     }
 
-    private fun alert(message: String) {
+    private fun alert(message: String, title: String? = null) {
         AlertDialog.Builder(this)
-            .setTitle(R.string.error_alert_title)
+            .setTitle(title ?: getString(R.string.error_alert_title))
             .setMessage(message)
             .create()
             .show()
@@ -152,148 +151,6 @@ class MainActivity : AppCompatActivity(), ContentAdapter.ContentAdapterListen {
         }
     }
 
-    class PageViewModel : ViewModel() {
-        private var requestJob: Job? = null
-        val state: MutableLiveData<State> by lazy { MutableLiveData<State>(State.IDLE) }
-        private var linesList = ArrayList<Line>()
-        val lines: MutableLiveData<List<Line>> by lazy { MutableLiveData<List<Line>>() }
-        val event: MutableLiveData<Event> by lazy { MutableLiveData<Event>() }
-
-        enum class State {
-            IDLE, CONNECTING, RECEIVING
-        }
-
-        abstract class Event(var handled: Boolean = false)
-        data class SuccessEvent(val uri: String) : Event()
-        data class RedirectEvent(val uri: String, val redirects: Int) : Event()
-        data class FailureEvent(val message: String) : Event()
-
-        /**
-         * Perform a request against this URI.
-         *
-         * The URI must be valid, absolute and with a gemini scheme.
-         */
-        fun sendGeminiRequest(uri: Uri, redirects: Int = 0) {
-            Log.d(TAG, "sendRequest: URI \"$uri\"")
-            state.postValue(State.CONNECTING)
-            requestJob?.apply { if (isActive) cancel() }
-            requestJob = viewModelScope.launch(Dispatchers.IO) {
-                val response = try {
-                    val request = Request(uri)
-                    val socket = request.connect()
-                    val channel = request.proceed(socket, this)
-                    Response.from(channel, viewModelScope)
-                } catch (e: Exception) {
-                    Log.e(TAG, "sendGeminiRequest coroutine: ${e.stackTraceToString()}")
-                    // If we got cancelled, die silently.
-                    if (!isActive)
-                        return@launch
-                    signalError(
-                        when (e) {
-                            is UnknownHostException -> "Unknown host \"${uri.authority}\"."
-                            is ConnectException -> "Can't connect to this server: ${e.localizedMessage}."
-                            is SocketTimeoutException -> "Connection timed out."
-                            is CancellationException -> "Connection cancelled: ${e.message}."
-                            else -> "Oops, something failed!"
-                        }
-                    )
-                    return@launch
-                }
-                if (!isActive)
-                    return@launch
-                if (response == null) {
-                    signalError("Can't parse server response.")
-                    return@launch
-                }
-
-                Log.i(TAG, "sendRequest: got ${response.code} with meta \"${response.meta}\"")
-                when (response.code.getCategory()) {
-                    Response.Code.Category.SUCCESS -> handleRequestSuccess(response, uri)
-                    Response.Code.Category.REDIRECT -> handleRedirect(
-                        response,
-                        redirects = redirects + 1
-                    )
-                    Response.Code.Category.SERVER_ERROR -> handleError(response)
-                    else -> signalError("Can't handle code ${response.code}.")
-                }
-            }
-        }
-
-        private fun signalError(message: String) {
-            event.postValue(FailureEvent(message))
-            state.postValue(State.IDLE)
-        }
-
-        private suspend fun handleRequestSuccess(response: Response, uri: Uri) {
-            state.postValue(State.RECEIVING)
-
-            linesList.clear()
-            lines.postValue(linesList)
-            val charset = Charset.defaultCharset()
-            var mainTitle: String? = null
-            var lastUpdate = System.currentTimeMillis()
-            var lastNumLines = 0
-            Log.d(TAG, "handleRequestSuccess: start parsing line data")
-            try {
-                val lineChannel = parseData(response.data, charset, viewModelScope)
-                while (!lineChannel.isClosedForReceive) {
-                    val lineChannelResult = withTimeout(100) { lineChannel.tryReceive() }
-                    lineChannelResult.onSuccess { line ->
-                        linesList.add(line)
-                        if (mainTitle == null && line is TitleLine && line.level == 1)
-                            mainTitle = line.text
-                    }
-
-                    // Throttle the recycler view updates to 100ms and new content only.
-                    if (linesList.size > lastNumLines) {
-                        val time = System.currentTimeMillis()
-                        if (time - lastUpdate >= 100) {
-                            lines.postValue(linesList)
-                            lastUpdate = time
-                            lastNumLines = linesList.size
-                        }
-                    }
-                }
-            } catch (e: CancellationException) {
-                Log.e(TAG, "handleRequestSuccess: coroutine cancelled: ${e.message}")
-                state.postValue(State.IDLE)
-                return
-            }
-            Log.d(TAG, "handleRequestSuccess: done parsing line data")
-            lines.postValue(linesList)
-
-            // We record the history entry here: it's nice because we have the main title available
-            // and we're already in a coroutine for database access.
-            History.record(uri.toString(), mainTitle)
-            event.postValue(SuccessEvent(uri.toString()))
-            state.postValue(State.IDLE)
-        }
-
-        private fun handleRedirect(response: Response, redirects: Int) {
-            event.postValue(RedirectEvent(response.meta, redirects))
-        }
-
-        private fun handleError(response: Response) {
-            event.postValue(
-                FailureEvent(
-                    when (response.code) {
-                        Response.Code.TEMPORARY_FAILURE -> "40: the server encountered a temporary failure."
-                        Response.Code.SERVER_UNAVAILABLE -> "41: the server is currently unavailable."
-                        Response.Code.CGI_ERROR -> "42: a CGI script encountered an error."
-                        Response.Code.PROXY_ERROR -> "43: the server failed to proxy the request."
-                        Response.Code.SLOW_DOWN -> "44: slow down!"
-                        Response.Code.PERMANENT_FAILURE -> "50: this request failed and similar requests will likely fail as well."
-                        Response.Code.NOT_FOUND -> "51: this page can't be found."
-                        Response.Code.GONE -> "52: this page is gone."
-                        Response.Code.PROXY_REQUEST_REFUSED -> "53: the server refused to proxy the request."
-                        Response.Code.BAD_REQUEST -> "59: bad request."
-                        else -> "${response.code}: unknown error code."
-                    }
-                )
-            )
-        }
-    }
-
     companion object {
         const val TAG = "MainActivity"
     }

app/src/main/java/dev/lowrespalmtree/comet/PageViewModel.kt

@@ -0,0 +1,183 @@
+package dev.lowrespalmtree.comet
+
+import android.net.Uri
+import android.util.Log
+import androidx.lifecycle.MutableLiveData
+import androidx.lifecycle.ViewModel
+import androidx.lifecycle.viewModelScope
+import kotlinx.coroutines.*
+import kotlinx.coroutines.channels.onSuccess
+import java.net.ConnectException
+import java.net.SocketTimeoutException
+import java.net.UnknownHostException
+import java.nio.charset.Charset
+
+@ExperimentalCoroutinesApi
+class PageViewModel : ViewModel() {
+    private var requestJob: Job? = null
+    val state: MutableLiveData<State> by lazy { MutableLiveData<State>(State.IDLE) }
+    private var linesList = ArrayList<Line>()
+    val lines: MutableLiveData<List<Line>> by lazy { MutableLiveData<List<Line>>() }
+    val event: MutableLiveData<Event> by lazy { MutableLiveData<Event>() }
+
+    enum class State {
+        IDLE, CONNECTING, RECEIVING
+    }
+
+    abstract class Event(var handled: Boolean = false)
+    data class SuccessEvent(val uri: String) : Event()
+    data class RedirectEvent(val uri: String, val redirects: Int) : Event()
+    data class FailureEvent(
+        val short: String,
+        val details: String,
+        val serverDetails: String? = null
+    ) : Event()
+
+    /**
+     * Perform a request against this URI.
+     *
+     * The URI must be valid, absolute and with a gemini scheme.
+     */
+    fun sendGeminiRequest(uri: Uri, redirects: Int = 0) {
+        Log.d(TAG, "sendRequest: URI \"$uri\"")
+        state.postValue(State.CONNECTING)
+        requestJob?.apply { if (isActive) cancel() }
+        requestJob = viewModelScope.launch(Dispatchers.IO) {
+            val response = try {
+                val request = Request(uri)
+                val socket = request.connect()
+                val channel = request.proceed(socket, this)
+                Response.from(channel, viewModelScope)
+            } catch (e: Exception) {
+                Log.e(TAG, "sendGeminiRequest coroutine: ${e.stackTraceToString()}")
+                // If we got cancelled, die silently.
+                if (!isActive)
+                    return@launch
+                signalError(
+                    when (e) {
+                        is UnknownHostException -> "Unknown host \"${uri.authority}\"."
+                        is ConnectException -> "Can't connect to this server: ${e.localizedMessage}."
+                        is SocketTimeoutException -> "Connection timed out."
+                        is CancellationException -> "Connection cancelled: ${e.message}."
+                        else -> "Oops, something failed!"
+                    }
+                )
+                return@launch
+            }
+            if (!isActive)
+                return@launch
+            if (response == null) {
+                signalError("Can't parse server response.")
+                return@launch
+            }
+
+            Log.i(TAG, "sendRequest: got ${response.code} with meta \"${response.meta}\"")
+            when (response.code.getCategory()) {
+                Response.Code.Category.SUCCESS ->
+                    handleRequestSuccess(response, uri)
+                Response.Code.Category.REDIRECT ->
+                    handleRedirect(response, redirects = redirects + 1)
+                Response.Code.Category.SERVER_ERROR, Response.Code.Category.CLIENT_ERROR ->
+                    handleError(response)
+                else ->
+                    signalError("Can't handle code ${response.code}.")
+            }
+        }
+    }
+
+    private fun signalError(message: String) {
+        event.postValue(FailureEvent("Error", message))
+        state.postValue(State.IDLE)
+    }
+
+    private suspend fun handleRequestSuccess(response: Response, uri: Uri) {
+        state.postValue(State.RECEIVING)
+
+        linesList.clear()
+        lines.postValue(linesList)
+        val charset = Charset.defaultCharset()
+        var mainTitle: String? = null
+        var lastUpdate = System.currentTimeMillis()
+        var lastNumLines = 0
+        Log.d(TAG, "handleRequestSuccess: start parsing line data")
+        try {
+            val lineChannel = parseData(response.data, charset, viewModelScope)
+            while (!lineChannel.isClosedForReceive) {
+                val lineChannelResult = withTimeout(100) { lineChannel.tryReceive() }
+                lineChannelResult.onSuccess { line ->
+                    linesList.add(line)
+                    if (mainTitle == null && line is TitleLine && line.level == 1)
+                        mainTitle = line.text
+                }
+
+                // Throttle the recycler view updates to 100ms and new content only.
+                if (linesList.size > lastNumLines) {
+                    val time = System.currentTimeMillis()
+                    if (time - lastUpdate >= 100) {
+                        lines.postValue(linesList)
+                        lastUpdate = time
+                        lastNumLines = linesList.size
+                    }
+                }
+            }
+        } catch (e: CancellationException) {
+            Log.e(TAG, "handleRequestSuccess: coroutine cancelled: ${e.message}")
+            state.postValue(State.IDLE)
+            return
+        }
+        Log.d(TAG, "handleRequestSuccess: done parsing line data")
+        lines.postValue(linesList)
+
+        // We record the history entry here: it's nice because we have the main title available
+        // and we're already in a coroutine for database access.
+        History.record(uri.toString(), mainTitle)
+        event.postValue(SuccessEvent(uri.toString()))
+        state.postValue(State.IDLE)
+    }
+
+    private fun handleRedirect(response: Response, redirects: Int) {
+        event.postValue(RedirectEvent(response.meta, redirects))
+    }
+
+    private fun handleError(response: Response) {
+        val briefMessage = when (response.code) {
+            Response.Code.TEMPORARY_FAILURE -> "40 Temporary failure"
+            Response.Code.SERVER_UNAVAILABLE -> "41 Server unavailable"
+            Response.Code.CGI_ERROR -> "42 CGI error"
+            Response.Code.PROXY_ERROR -> "43 Proxy error"
+            Response.Code.SLOW_DOWN -> "44 Slow down"
+            Response.Code.PERMANENT_FAILURE -> "50 Permanent failure"
+            Response.Code.NOT_FOUND -> "51 Not found"
+            Response.Code.GONE -> "52 Gone"
+            Response.Code.PROXY_REQUEST_REFUSED -> "53 Proxy request refused"
+            Response.Code.BAD_REQUEST -> "59 Bad request"
+            else -> "${response.code} (unknown)"
+        }
+        val longMessage: String
+        var serverMessage: String? = null
+        if (response.code == Response.Code.SLOW_DOWN) {
+            longMessage =
+                "You should wait ${response.meta.toIntOrNull() ?: "a few"} seconds before retrying."
+        } else {
+            longMessage = when (response.code) {
+                Response.Code.TEMPORARY_FAILURE -> "The server encountered a temporary failure."
+                Response.Code.SERVER_UNAVAILABLE -> "The server is currently unavailable."
+                Response.Code.CGI_ERROR -> "A CGI script encountered an error."
+                Response.Code.PROXY_ERROR -> "The server failed to proxy the request."
+                Response.Code.PERMANENT_FAILURE -> "This request failed and similar requests will likely fail as well."
+                Response.Code.NOT_FOUND -> "This page can't be found."
+                Response.Code.GONE -> "This page is gone."
+                Response.Code.PROXY_REQUEST_REFUSED -> "The server refused to proxy the request."
+                Response.Code.BAD_REQUEST -> "Bad request."
+                else -> "Unknown error code."
+            }
+            if (response.meta.isNotEmpty())
+                serverMessage = response.meta
+        }
+        event.postValue(FailureEvent(briefMessage, longMessage, serverMessage))
+    }
+
+    companion object {
+        const val TAG = "PageViewModel"
+    }
+}

app/src/main/java/dev/lowrespalmtree/comet/Response.kt

@@ -51,6 +51,8 @@ class Response(val code: Code, val meta: String, val data: Channel<ByteArray>) {
 
     companion object {
         private const val TAG = "Response"
+        private const val MAX_META_LEN = 1024
+        private const val MAX_HEADER_LEN = 2 + 1 + MAX_META_LEN + 2
 
         /** Return a response object from the incoming server data, served through the channel. */
         suspend fun from(channel: Channel<ByteArray>, scope: CoroutineScope): Response? {
@@ -62,7 +64,11 @@ class Response(val code: Code, val meta: String, val data: Channel<ByteArray>) {
                 val data = try {
                     channel.receive()
                 } catch (e: ClosedReceiveChannelException) {
-                    Log.d(TAG, "companion from: channel closed during initial receive")
+                    Log.i(TAG, "companion from: channel closed during initial receive")
+                    return null
+                }
+                if (received + data.size > MAX_HEADER_LEN) {
+                    Log.i(TAG, "companion from: received too much data for a valid header")
                     return null
                 }
                 // Push some data into our buffer.
@@ -80,7 +86,7 @@ class Response(val code: Code, val meta: String, val data: Channel<ByteArray>) {
             val bytes = headerBuffer.array()
             val headerData = bytes.sliceArray(0 until lfIndex)
             val (code, meta) = parseHeader(headerData)
-                ?: return null.also { Log.e(TAG, "companion from: can't parse header") }
+                ?: return null .also { Log.i(TAG, "companion from: can't parse header") }
             val response = Response(code, meta, Channel())
             scope.launch {
                 // If we got too much data from the channel: push the trailing data first.