Skip to content

Show error in UI when update field fails #430

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

Merged
merged 1 commit into from
Apr 9, 2025
Merged

Show error in UI when update field fails #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ jooq = "3.18.25"
kotlin = "2.0.21"
kotlinxCoroutines = "1.10.1"
ktlint = "0.47.1"
misk = "2025.04.04.132805-43bcba0"
misk = "2025.04.09.202256-90dc722"
okhttp = "5.0.0-alpha.14"
sqldelight = "2.0.2"
wire = "5.2.1"
Expand Down
2 changes: 1 addition & 1 deletion service/src/main/kotlin/app/cash/backfila/ui/actions/BackfillCreateHandlerAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class BackfillCreateHandlerAction @Inject constructor(
val errorHtmlResponseBody = dashboardPageLayout.newBuilder()
.buildHtmlResponseBody {
div("py-20") {
AlertError(message = "Backfill Create Failed: $e", label = "Try Again", onClick = "history.back(); return false;")
AlertError(message = "Backfill create or clone failed: $e", label = "Try Again", onClick = "history.back(); return false;")
}
}
return Response(
Expand Down
88 changes: 56 additions & 32 deletions service/src/main/kotlin/app/cash/backfila/ui/actions/BackfillShowButtonHandlerAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ import app.cash.backfila.dashboard.StopBackfillRequest
import app.cash.backfila.dashboard.UpdateBackfillAction
import app.cash.backfila.dashboard.UpdateBackfillRequest
import app.cash.backfila.service.persistence.BackfillState
import javax.inject.Inject
import javax.inject.Singleton
import app.cash.backfila.ui.components.AlertError
import app.cash.backfila.ui.components.DashboardPageLayout
import kotlinx.html.div
import misk.security.authz.Authenticated
import misk.web.Get
import misk.web.PathParam
Expand All @@ -20,9 +21,12 @@ import misk.web.actions.WebAction
import misk.web.mediatype.MediaTypes
import misk.web.toResponseBody
import okhttp3.Headers
import javax.inject.Inject
import javax.inject.Singleton

@Singleton
class BackfillShowButtonHandlerAction @Inject constructor(
private val dashboardPageLayout: DashboardPageLayout,
private val startBackfillAction: StartBackfillAction,
private val stopBackfillAction: StopBackfillAction,
private val updateBackfillAction: UpdateBackfillAction,
Expand All @@ -35,43 +39,63 @@ class BackfillShowButtonHandlerAction @Inject constructor(
@QueryParam field_id: String?,
@QueryParam field_value: String?,
): Response<ResponseBody> {
if (!field_id.isNullOrBlank()) {
when (field_id) {
"state" -> {
if (field_value == BackfillState.PAUSED.name) {
stopBackfillAction.stop(id.toLong(), StopBackfillRequest())
} else if (field_value == BackfillState.RUNNING.name) {
startBackfillAction.start(id.toLong(), StartBackfillRequest())
try {
if (!field_id.isNullOrBlank()) {
when (field_id) {
"state" -> {
if (field_value == BackfillState.PAUSED.name) {
stopBackfillAction.stop(id.toLong(), StopBackfillRequest())
} else if (field_value == BackfillState.RUNNING.name) {
startBackfillAction.start(id.toLong(), StartBackfillRequest())
}
}
}
"num_threads" -> {
val numThreads = field_value?.toIntOrNull()
if (numThreads != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(num_threads = numThreads))

"num_threads" -> {
val numThreads = field_value?.toIntOrNull()
if (numThreads != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(num_threads = numThreads))
}
}
}
"scan_size" -> {
val scanSize = field_value?.toLongOrNull()
if (scanSize != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(scan_size = scanSize))

"scan_size" -> {
val scanSize = field_value?.toLongOrNull()
if (scanSize != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(scan_size = scanSize))
}
}
}
"batch_size" -> {
val batchSize = field_value?.toLongOrNull()
if (batchSize != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(batch_size = batchSize))

"batch_size" -> {
val batchSize = field_value?.toLongOrNull()
if (batchSize != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(batch_size = batchSize))
}
}
}
"extra_sleep_ms" -> {
val extraSleepMs = field_value?.toLongOrNull()
if (extraSleepMs != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(extra_sleep_ms = extraSleepMs))

"extra_sleep_ms" -> {
val extraSleepMs = field_value?.toLongOrNull()
if (extraSleepMs != null) {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(extra_sleep_ms = extraSleepMs))
}
}

"backoff_schedule" -> {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(backoff_schedule = field_value))
}
}
"backoff_schedule" -> {
updateBackfillAction.update(id.toLong(), UpdateBackfillRequest(backoff_schedule = field_value))
}
}
} catch (e: Exception) {
// Since this action is only hit from the UI, catch any validation errors and show them to the user
val errorHtmlResponseBody = dashboardPageLayout.newBuilder()
.buildHtmlResponseBody {
div("py-20") {
AlertError(message = "Update backfill field failed: $e", label = "Try Again", onClick = "history.back(); return false;")
}
}
return Response(
body = errorHtmlResponseBody,
statusCode = 200,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

statusCode 200 for an exception? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes because this endpoint is only serving browser traffic

headers = Headers.headersOf("Content-Type", MediaTypes.TEXT_HTML),
)
}

return Response(
Expand Down