improve errors for CEL input

This commit is contained in:
aecsocket
2026-08-12 08:12:44 +00:00
parent 7f59feeb98
commit 04d00c0ba0
10 changed files with 183 additions and 109 deletions
@@ -2013,23 +2013,20 @@ function copyId() {
</div>
<div class="detail-verdict-action-groups">
<ButtonStyled>
<button
type="button"
:disabled="copyingCelDetails.has(flag.detail.id)"
@click="copyDetailCelInput(flag.detail.id)"
>
<LoaderCircleIcon
v-if="copyingCelDetails.has(flag.detail.id)"
class="animate-spin"
aria-hidden="true"
/>
<ClipboardCopyIcon v-else aria-hidden="true" />
<span aria-live="polite">
{{ copiedCelDetails.has(flag.detail.id) ? 'Copied!' : 'Copy CEL' }}
</span>
</button>
</ButtonStyled>
<Button
:disabled="copyingCelDetails.has(flag.detail.id)"
@click="copyDetailCelInput(flag.detail.id)"
>
<LoaderCircleIcon
v-if="copyingCelDetails.has(flag.detail.id)"
class="animate-spin"
aria-hidden="true"
/>
<ClipboardCopyIcon v-else aria-hidden="true" />
<span aria-live="polite">
{{ copiedCelDetails.has(flag.detail.id) ? 'Copied!' : 'Copy CEL' }}
</span>
</Button>
<div
class="detail-verdict-buttons"
role="group"
@@ -10,7 +10,7 @@ import {
SortDescIcon,
} from '@modrinth/assets'
import {
ButtonStyled,
ButtonLink,
Combobox,
type ComboboxOption,
commonMessages,
@@ -654,12 +654,10 @@ onUnmounted(() => {
<div
class="flex flex-col items-stretch justify-end gap-2 sm:flex-row sm:items-center lg:flex-shrink-0"
>
<ButtonStyled>
<NuxtLink to="/moderation/technical-review/rules">
<SettingsIcon class="size-5" />
Rules
</NuxtLink>
</ButtonStyled>
<ButtonLink to="/moderation/technical-review/rules">
<SettingsIcon class="size-5" />
Rules
</ButtonLink>
<Combobox
v-model="currentResponseFilter"
@@ -94,9 +94,19 @@
<div
v-if="ruleTestError"
class="border-red/40 rounded-lg border bg-highlight-red p-3 text-sm text-red"
role="alert"
class="border-red/40 max-h-64 overflow-auto rounded-lg border bg-highlight-red p-3 text-red"
>
{{ ruleTestError }}
<code class="rule-test-error">
<span>{{ ruleTestError.summary }}</span>
<span
v-for="(detail, index) in ruleTestError.details"
:key="index"
class="rule-test-error-detail"
>
{{ detail }}
</span>
</code>
</div>
<div
@@ -160,14 +170,10 @@
</section>
<div class="flex justify-end gap-2">
<ButtonStyled>
<button type="button" @click="closeRuleModal">Cancel</button>
</ButtonStyled>
<ButtonStyled color="brand">
<button type="submit" :disabled="isSaving">
{{ isSaving ? 'Saving...' : 'Save rule' }}
</button>
</ButtonStyled>
<Button @click="closeRuleModal">Cancel</Button>
<Button type="colored" color="brand" native-type="submit" :disabled="isSaving">
{{ isSaving ? 'Saving...' : 'Save rule' }}
</Button>
</div>
</form>
</NewModal>
@@ -192,11 +198,14 @@
<div class="flex flex-col gap-6">
<div class="flex flex-wrap items-center justify-between gap-3">
<div class="flex items-center gap-3">
<ButtonStyled circular type="transparent">
<NuxtLink to="/moderation/technical-review" aria-label="Back to tech review queue">
<ArrowLeftIcon />
</NuxtLink>
</ButtonStyled>
<ButtonLink
to="/moderation/technical-review"
type="quiet"
class="!size-10 !rounded-full !p-0"
aria-label="Back to tech review queue"
>
<ArrowLeftIcon />
</ButtonLink>
<div>
<h1 class="m-0 text-2xl font-bold text-contrast">Delphi rules</h1>
<p class="m-0 text-secondary">Transform or hide Delphi issue traces.</p>
@@ -204,18 +213,14 @@
</div>
<div class="flex flex-wrap gap-2">
<ButtonStyled>
<button type="button" :disabled="isScanning" @click="scanModal?.show()">
<PlayIcon />
{{ isScanning ? 'Scanning...' : 'Run full scan' }}
</button>
</ButtonStyled>
<ButtonStyled color="brand">
<button type="button" :disabled="isScanning" @click="openCreateModal">
<PlusIcon />
Create rule
</button>
</ButtonStyled>
<Button :disabled="isScanning" @click="scanModal?.show()">
<PlayIcon />
{{ isScanning ? 'Scanning...' : 'Run full scan' }}
</Button>
<Button type="colored" color="brand" :disabled="isScanning" @click="openCreateModal">
<PlusIcon />
Create rule
</Button>
</div>
</div>
@@ -247,9 +252,7 @@
</div>
<div v-else-if="loadFailed" class="universal-card flex flex-col items-center gap-3 py-8">
<p class="m-0 text-secondary">Failed to load Delphi rules.</p>
<ButtonStyled>
<button type="button" @click="loadRules">Try again</button>
</ButtonStyled>
<Button @click="loadRules">Try again</Button>
</div>
<EmptyState
v-else-if="rules.length === 0"
@@ -271,18 +274,19 @@
</p>
</div>
<div class="flex gap-2">
<ButtonStyled>
<button type="button" :disabled="isScanning" @click="openEditModal(rule)">
<EditIcon />
Edit
</button>
</ButtonStyled>
<ButtonStyled color="red">
<button type="button" :disabled="isScanning" @click="openDeleteModal(rule)">
<TrashIcon />
Delete
</button>
</ButtonStyled>
<Button :disabled="isScanning" @click="openEditModal(rule)">
<EditIcon />
Edit
</Button>
<Button
type="colored"
color="red"
:disabled="isScanning"
@click="openDeleteModal(rule)"
>
<TrashIcon />
Delete
</Button>
</div>
</div>
<pre
@@ -352,40 +356,28 @@
/>
</p>
</div>
<ButtonStyled>
<NuxtLink v-if="detail.project_id" :to="getAffectedDetailLink(detail)">
<ExternalIcon />
View
</NuxtLink>
<button
v-else
type="button"
disabled
title="This trace is not attached to a project"
>
<ExternalIcon />
View
</button>
</ButtonStyled>
<ButtonLink v-if="detail.project_id" :to="getAffectedDetailLink(detail)">
<ExternalIcon />
View
</ButtonLink>
<Button v-else disabled title="This trace is not attached to a project">
<ExternalIcon />
View
</Button>
</div>
<div
v-if="rule.affected_details_count > 3"
class="relative z-20 mt-1 flex justify-center"
>
<ButtonStyled circular type="transparent">
<button
type="button"
:disabled="loadingAffectedRuleIds.has(rule.id)"
@click="toggleAffectedDetails(rule)"
>
<LoaderCircleIcon
v-if="loadingAffectedRuleIds.has(rule.id)"
class="animate-spin"
/>
{{ expandedAffectedDetails.has(rule.id) ? 'Show less' : 'Show more' }}
</button>
</ButtonStyled>
<Button
type="quiet"
:disabled="loadingAffectedRuleIds.has(rule.id)"
@click="toggleAffectedDetails(rule)"
>
<LoaderCircleIcon v-if="loadingAffectedRuleIds.has(rule.id)" class="animate-spin" />
{{ expandedAffectedDetails.has(rule.id) ? 'Show less' : 'Show more' }}
</Button>
</div>
</div>
</section>
@@ -400,7 +392,7 @@
</template>
<script setup lang="ts">
import { type Labrinth, SseParser } from '@modrinth/api-client'
import { type Labrinth, ModrinthServerError, SseParser } from '@modrinth/api-client'
import {
ArrowLeftIcon,
EditIcon,
@@ -413,7 +405,8 @@ import {
} from '@modrinth/assets'
import {
Avatar,
ButtonStyled,
Button,
ButtonLink,
ConfirmModal,
EmptyState,
injectModrinthClient,
@@ -437,6 +430,11 @@ const RULE_EDITOR_OPTIONS: Partial<Ace.EditorOptions> = {
useSoftTabs: true,
}
type RuleTestError = {
summary: string
details: string[]
}
const TEST_INPUTS: Labrinth.TechReview.Internal.RuleInput[] = [
{
schema_version: 1,
@@ -532,7 +530,7 @@ const ruleSchema = ref<Labrinth.TechReview.Internal.DelphiRuleSchemaResponse | n
const editingRuleId = ref<number | null>(null)
const ruleToDelete = ref<Labrinth.TechReview.Internal.DelphiRule | null>(null)
const ruleTestEffects = ref<Array<Labrinth.TechReview.Internal.DelphiRuleEffect | null>>([])
const ruleTestError = ref<string | null>(null)
const ruleTestError = ref<RuleTestError | null>(null)
const scanProgress = ref<Labrinth.TechReview.Internal.DelphiRuleScanEvent | null>(null)
const expandedAffectedDetails = reactive(
new Map<number, Labrinth.TechReview.Internal.DelphiRuleAffectedDetail[]>(),
@@ -711,7 +709,10 @@ async function testRule() {
if (!rule) {
isTestingRule.value = false
ruleTestError.value = 'Enter a CEL expression to test it.'
ruleTestError.value = {
summary: 'Enter a CEL expression to test it.',
details: [],
}
return
}
@@ -727,7 +728,14 @@ async function testRule() {
} catch (error) {
if (requestId !== ruleTestRequestId) return
ruleTestError.value = error instanceof Error ? error.message : 'The rule could not be tested.'
const details =
error instanceof ModrinthServerError && Array.isArray(error.v1Error?.details)
? error.v1Error.details.filter((detail): detail is string => typeof detail === 'string')
: []
ruleTestError.value = {
summary: error instanceof Error ? error.message : 'The rule could not be tested.',
details,
}
} finally {
if (requestId === ruleTestRequestId) {
isTestingRule.value = false
@@ -1009,3 +1017,22 @@ async function runFullScan() {
onMounted(loadRules)
onUnmounted(() => scanAbortController?.abort())
</script>
<style scoped>
.rule-test-error {
display: block;
font-family: monospace;
font-size: 0.75rem;
line-height: 1.625;
overflow-wrap: anywhere;
white-space: pre-wrap;
}
.rule-test-error > span {
display: block;
}
.rule-test-error-detail {
margin-inline-start: 1rem;
}
</style>
@@ -1417,7 +1417,7 @@ pub async fn update_issue_details(
security(("bearer_auth" = [])),
responses((status = NO_CONTENT))
)]
#[post("/global-issue-detail")]
#[post("/global-traces")]
pub async fn update_global_issue_details(
req: HttpRequest,
pool: web::Data<PgPool>,
@@ -133,7 +133,7 @@ pub struct GlobalIssueDetailTrace {
security(("bearer_auth" = [])),
responses((status = OK, body = SearchGlobalIssueDetailsResponse))
)]
#[post("/global-issue-detail/search")]
#[post("/global-traces/search")]
pub async fn search_global_issue_details(
req: HttpRequest,
pool: web::Data<PgPool>,
@@ -357,7 +357,7 @@ pub async fn search_global_issue_details(
security(("bearer_auth" = [])),
responses((status = OK, body = GetGlobalIssueDetailResponse))
)]
#[post("/global-issue-detail/local-traces")]
#[post("/global-traces/local-traces")]
pub async fn get_global_issue_detail(
req: HttpRequest,
pool: web::Data<PgPool>,
@@ -115,7 +115,9 @@ impl WriteDelphiRule {
.await
.wrap_internal_err("failed to join cel compilation task")?
.map_err(|error| {
ApiError::Request(eyre!("invalid cel expression: {error}"))
ApiError::Request(
eyre!(error).wrap_err("invalid cel expression"),
)
})?;
Ok(ValidatedRule {
@@ -165,7 +167,9 @@ pub async fn test_rule(
.await
.wrap_internal_err("failed to join cel compilation task")?
.map_err(|error| {
ApiError::Request(eyre!("invalid cel expression: {error}"))
ApiError::Request(
eyre!(error).wrap_err("invalid cel expression"),
)
})?;
let mut effects = Vec::with_capacity(request.inputs.len());
+41 -2
View File
@@ -235,6 +235,23 @@ impl ApiError {
}
pub fn as_api_error<'a>(&self) -> crate::models::error::ApiError<'a> {
let report = match self {
Self::Internal(report)
| Self::Request(report)
| Self::Auth(report)
| Self::NotFound(report)
| Self::Conflict(report)
| Self::FailedDependency(report)
| Self::PreconditionRequired(report)
| Self::PreconditionFailed(report)
| Self::RateLimit(report) => report,
};
let details = report
.chain()
.skip(1)
.map(ToString::to_string)
.collect::<Vec<_>>();
crate::models::error::ApiError {
error: match self {
Self::Internal(..) => "internal_error",
@@ -247,8 +264,8 @@ impl ApiError {
Self::PreconditionFailed(..) => "precondition_failed",
Self::RateLimit(..) => "ratelimit_error",
},
description: format!("{self:#}"),
details: None,
description: report.to_string(),
details: (!details.is_empty()).then(|| serde_json::json!(details)),
}
}
}
@@ -272,3 +289,25 @@ impl actix_web::ResponseError for ApiError {
HttpResponse::build(self.status_code()).json(self.as_api_error())
}
}
#[cfg(test)]
mod tests {
use super::ApiError;
#[test]
fn api_error_serializes_source_chain_as_details() {
let error = ApiError::Request(
eyre::eyre!("root cause")
.wrap_err("intermediate context")
.wrap_err("request failed"),
);
let response = error.as_api_error();
assert_eq!(response.description, "request failed");
assert_eq!(
response.details,
Some(serde_json::json!(["intermediate context", "root cause"])),
);
}
}
@@ -227,7 +227,7 @@ export class LabrinthTechReviewInternalModule extends AbstractModule {
public async updateGlobalIssueDetails(
data: Labrinth.TechReview.Internal.UpdateGlobalIssueRequest[],
): Promise<void> {
return this.client.request<void>('/moderation/tech-review/global-issue-detail', {
return this.client.request<void>('/moderation/tech-review/global-traces', {
api: 'labrinth',
version: 'internal',
method: 'POST',
@@ -239,7 +239,7 @@ export class LabrinthTechReviewInternalModule extends AbstractModule {
params: Labrinth.TechReview.Internal.SearchGlobalIssueDetailsRequest,
): Promise<Labrinth.TechReview.Internal.SearchGlobalIssueDetailsResponse> {
return this.client.request<Labrinth.TechReview.Internal.SearchGlobalIssueDetailsResponse>(
'/moderation/tech-review/global-issue-detail/search',
'/moderation/tech-review/global-traces/search',
{
api: 'labrinth',
version: 'internal',
@@ -253,7 +253,7 @@ export class LabrinthTechReviewInternalModule extends AbstractModule {
params: Labrinth.TechReview.Internal.GetGlobalIssueDetailRequest,
): Promise<Labrinth.TechReview.Internal.GetGlobalIssueDetailResponse> {
return this.client.request<Labrinth.TechReview.Internal.GetGlobalIssueDetailResponse>(
'/moderation/tech-review/global-issue-detail/local-traces',
'/moderation/tech-review/global-traces/local-traces',
{
api: 'labrinth',
version: 'internal',
+5
View File
@@ -38,6 +38,11 @@ export type ModrinthErrorResponse = {
*/
description: string
/**
* Structured details about the error
*/
details?: unknown
/**
* Optional context about where the error occurred
*/
+4
View File
@@ -0,0 +1,4 @@
#!/usr/bin/env bash
source .env
psql $DATABASE_URL < fixtures/labrinth-seed-data-202508052143.sql