elastic / elastic/docs-builder
[Feature Request]: Improve error messages for applies-to problems
- Dominant language
- C#
- Stars
- 24
- Forks
- 44
- Avg merge
- 1d 7h
- Merged PRs (30d)
- 146
Description
### Prerequisites
- [x] I have searched existing issues to ensure this feature hasn't already been requested
- [x] I have tested using the latest version of docs-builder
### What problem are you trying to solve?
The error messages that occur when there's an applies-to formatting error are very unhelpful.
For example in https://github.com/elastic/elasticsearch/pull/144885 the message that appeared in the CI failure was:
```
Error: Unhandled IsolatedBuildService exception: While scanning a plain scalar value, found invalid mapping.
(Line: 1, Col: 26, Idx: 25) - (Line: 1, Col: 26, Idx: 25): While scanning a plain scalar value, found invalid mapping.
```
The actual fix was https://github.com/elastic/elasticsearch/pull/144885/changes/43de107169c00bbba4939506c20f0d3c42c8e8cf
### Proposed Solution
Add more detail to help pinpoint the problem. Here's an example of a quick AI-generated plan:
# Plan
Improved `{applies-item}` YAML error messages
## Problem
- Invalid one-line forms such as `stack: ga 9.4, serverless: ga` are not valid YAML as a single scalar; the fix in the linked PR uses a JSON object: `{ "stack": "ga 9.4", "serverless": "ga" }`.
- Today, `AppliesItemBlock.FinalizeAndValidate` in `src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs` calls `**GenerateSyncKey` before `ParseApplicableTo**` (lines 62–68). A bad argument can therefore fail during sync-key deserialization first, which complicates diagnostics and can surface as a raw YamlDotNet message without actionable guidance.
- `YamlSerialization.Deserialize` in `src/Elastic.Markdown/Myst/YamlSerialization.cs` already wraps `YamlException` in `InvalidOperationException` with a `yamlSourceContext` prefix, but does not add applicability-specific hints.
## Approach
### 1. Reorder `FinalizeAndValidate` (directive behavior)
In `AppliesSwitchBlock.cs` (`src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs`):
- Run `**ParseApplicableTo` first** (when `AppliesToDefinition` is non-empty).
- Set `**SyncKey`** to `Prop("sync") ??` a new helper that hashes from `**ApplicableTo**` when `AppliesTo` is non-null (e.g. `GenerateSyncKeyFromApplicable(ApplicableTo)`), mirroring the existing `ShortId.Create(applicableTo.ToString())` logic in `[GenerateSyncKey](src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs)` (lines 93–99).
- Keep the **public** `GenerateSyncKey(string, ProductsConfiguration)` used by tests: implement as deserialize + `GenerateSyncKeyFromApplicable` so behavior stays centralized.
**Edge case:** If parsing fails and returns null, `SyncKey` may be null unless `sync` prop is set—acceptable for invalid tabs; document in code comment if tests need adjustment.
### 2. Contextual hint in `YamlSerialization` (optional, targeted)
In `[YamlSerialization.cs](src/Elastic.Markdown/Myst/YamlSerialization.cs)`, inside the `YamlException` catch when `yamlSourceContext` is non-null:
- Only when the **target type is `ApplicableTo`** (use `typeof(T) == typeof(ApplicableTo)` or a dedicated overload—prefer **minimal API surface**: branch inside existing generic method) **and** `yamlSourceContext` contains `applies-item` (case-insensitive).
- Append a **short hint** when both:
- The exception message suggests structural YAML issues (e.g. contains `invalid mapping` or `plain scalar`), **and**
- The input string **looks like comma-separated top-level keys** (heuristic: regex such as `,\s*\w[\w-]*\s*:`), which distinguishes:
- **Bad:** `stack: ga 9.4, serverless: ga`
- **Still valid** patterns like `stack: preview 9.0, ga 9.1` (comma without `, word:`) used in `[ApplicabilitySwitchTests](tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs)` (`NormalizesSyncKeyOrder`).
**Hint copy (concise, style-neutral):** Explain that multiple keys on one line should use a **JSON object** or a **braced YAML mapping**; avoid implying that *all* commas are wrong.
### 3. Directive-level message polish (optional)
In `ParseApplicableTo`, optionally prefix `EmitError` with a clearer title, e.g. `Unable to parse {applies-item} applicability` plus the raw argument snippet, so diagnostics in the panel read better than a bare YamlDotNet line. The inner exception (with hint from step 2) remains attached.
### 4. Documentation (optional, small)
In `docs/syntax/applies-switch.md`, add a short **“Troubleshooting”** or **“Multiple keys on one line”** note: prefer JSON or `{ key: value, ... }` braced form when specifying several keys—aligned with existing “Multiple applies_to definitions” section (lines 41–77) so it does not duplicate the whole page.
### 5. Tests
- Add or extend a test in `ApplicabilitySwitchTests`(`tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs`) (or a small dedicated test file) that feeds an **invalid** comma-separated two-key string and asserts the emitted diagnostic **includes** the hint substring (or that `InvalidOperationException.Message` contains it if testing at `YamlSerialization` level).
- Run existing `**GenerateSyncKey`** / `**NormalizesSyncKeyOrder**` tests to ensure no regression.
## Files to touch
| File | Change |
| ------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------ |
| `[src/Elastic.Markdown/Myst/YamlSerialization.cs](src/Elastic.Markdown/Myst/YamlSerialization.cs)` | ApplicableTo + applies-item hint logic after `YamlException` |
| `[src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs](src/Elastic.Markdown/Myst/Directives/AppliesSwitch/AppliesSwitchBlock.cs)` | Reorder finalize; `GenerateSyncKeyFromApplicable`; refactor public `GenerateSyncKey` |
| `[docs/syntax/applies-switch.md](docs/syntax/applies-switch.md)` | Optional short troubleshooting blurb |
| `[tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs](tests/Elastic.Markdown.Tests/Directives/ApplicabilitySwitchTests.cs)` | New assertion for hint / invalid input |
## Out of scope
- Changing the `ApplicableTo` schema or Markdig directive syntax.
- Broad hints for every YAML failure site (front matter, settings YAML, etc.)—keep this scoped to `**ApplicableTo**` + `**applies-item**` context unless you explicitly expand later.
### Examples and Research
_No response_
### Alternative Solutions
_No response_
### Additional Context
_No response_
### How important is this feature to you?
Nice to have
Contributor guide
Assessment
This issue has not been assessed yet.