Skip to content

Commit

Permalink
Fix YAML Config Editing Console Runtime Errors by Switching to Yaml L…
Browse files Browse the repository at this point in the history
…ibrary (#289)

* fix: console errors and use yaml instead of jsYaml

* fix: minor error validation console fix

* fix: sonarCloud issue

* fix: full error message for Yaml errors

* fix: comments added for the YAML.parse try and catch
  • Loading branch information
roshan-gh authored Jan 22, 2024
1 parent 9bf4b1e commit d491dec
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 46 deletions.
3 changes: 1 addition & 2 deletions packages/otelbin/src/components/monaco-editor/Editor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ export default function Editor({ locked, setLocked }: { locked: boolean; setLock
}
}, [currentConfig, editorRef, monacoRef, serverSideValidationResult]);

const isValidConfig =
totalValidationErrors.jsYamlError == null && (totalValidationErrors.ajvErrors?.length ?? 0) === 0;
const isValidConfig = totalValidationErrors.yamlError == null && (totalValidationErrors.ajvErrors?.length ?? 0) === 0;

const handleEditorChange: OnChange = (value) => {
const configType = selectConfigType(value ?? "");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,13 @@
import { useEffect, useState, useRef } from "react";
import { ChevronDown, XCircle, AlertTriangle } from "lucide-react";
import { type NextFont } from "next/dist/compiled/@next/font";
import type { YAMLParseError } from "yaml";

export interface IAjvError {
message: string;
line?: number | null;
}

export interface IJsYamlError {
mark: {
line: number | null;
};
reason: string | null;
}

export interface IServerSideError {
message: string;
error: string;
Expand All @@ -25,7 +19,7 @@ export interface IServerSideError {
}

export interface IError {
jsYamlError?: IJsYamlError;
yamlError?: YAMLParseError;
ajvErrors?: IAjvError[];
customErrors?: string[];
customWarnings?: string[];
Expand All @@ -43,7 +37,7 @@ export default function ValidationErrorConsole({ errors, font }: { errors?: IErr
const [height, setHeight] = useState(170);
const errorCount =
(errors?.ajvErrors?.length ?? 0) +
(errors?.jsYamlError != null ? 1 : 0) +
(errors?.yamlError != null ? 1 : 0) +
(errors?.customErrors?.length ?? 0) +
(errors?.serverSideError?.error ? 1 : 0);

Expand Down Expand Up @@ -150,7 +144,7 @@ export default function ValidationErrorConsole({ errors, font }: { errors?: IErr
errors.customErrors.map((error: string, index: number) => {
return <ErrorMessage key={index} customErrors={error} font={font} />;
})}
{errors?.jsYamlError?.mark?.line && <ErrorMessage jsYamlError={errors?.jsYamlError} font={font} />}
{errors?.yamlError?.linePos?.[0] && <ErrorMessage yamlError={errors?.yamlError} font={font} />}
</div>
)}
</div>
Expand All @@ -161,14 +155,14 @@ export default function ValidationErrorConsole({ errors, font }: { errors?: IErr

export function ErrorMessage({
ajvError,
jsYamlError,
yamlError,
serverSideError,
customErrors,
customWarnings,
font,
}: {
ajvError?: IAjvError;
jsYamlError?: IJsYamlError;
yamlError?: YAMLParseError;
serverSideError?: IServerSideError;
customErrors?: string;
customWarnings?: string;
Expand All @@ -195,11 +189,9 @@ export function ErrorMessage({
<p className="max-w-[100%]">{`${customErrors}`}</p>
</div>
)}
{jsYamlError ? (
{yamlError ? (
<div className={`${font.className} ${errorsStyle}`}>
<p className="max-w-[100%]">{`${jsYamlError.reason} ${
jsYamlError.mark && `(Line ${jsYamlError.mark.line})`
}`}</p>
<p className="max-w-[100%]">{`${yamlError.message}`}</p>
</div>
) : (
<></>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from "./parseYaml";
import { capitalize, customValidate, findErrorElement } from "./otelCollectorConfigValidation";
import type { editor } from "monaco-editor";
import JsYaml, { FAILSAFE_SCHEMA } from "js-yaml";
import YAML from "yaml";

const editorBinding = {
prefix: "",
Expand Down Expand Up @@ -261,12 +261,12 @@ describe("findErrorElement", () => {
});
});

describe("JsYaml.load", () => {
describe("Yaml.parse", () => {
it("should load a YAML document that included numbers and convert it to JSON data with numbers as string", () => {
const jsonData = JsYaml.load(yamlData.fallback, { schema: FAILSAFE_SCHEMA });
const jsonData = YAML.parse(yamlData.fallback, { schema: "failsafe" });
expect(jsonData).toStrictEqual({
receivers: { otlp: null, 2222: null },
processors: { batch: null, 3333: null },
receivers: { otlp: "", 2222: "" },
processors: { batch: "", 3333: "" },
service: {
extensions: ["health_check", "pprof", "zpages", "999"],
pipelines: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// SPDX-License-Identifier: Apache-2.0

import { schema } from "./JSONSchema";
import type { IAjvError, IError, IJsYamlError } from "./ValidationErrorConsole";
import JsYaml, { FAILSAFE_SCHEMA } from "js-yaml";
import type { IAjvError, IError } from "./ValidationErrorConsole";
import YAML, { type YAMLParseError } from "yaml";
import Ajv from "ajv";
import type { ErrorObject } from "ajv";
import type { RefObject } from "react";
Expand Down Expand Up @@ -52,7 +52,7 @@ export function validateOtelCollectorConfigurationAndSetMarkers(
const serverSideValidationPath = serverSideValidationResult?.result?.path ?? [];

try {
const jsonData = JsYaml.load(configData, { schema: FAILSAFE_SCHEMA });
const jsonData = YAML.parse(configData, { logLevel: "error", schema: "failsafe" });
const valid = ajv.validate(schema, jsonData);
if (!valid) {
const errors = ajv.errors;
Expand All @@ -75,32 +75,27 @@ export function validateOtelCollectorConfigurationAndSetMarkers(
severity: 8,
message: errorInfo.message,
});
if (error instanceof JsYaml.YAMLException) {
errorInfo.line = error.mark.line + 1;
errorInfo.column = error.mark.column + 1;
}
return errorInfo;
});
ajvError.push(...validationErrors);
}
}
} catch (error: unknown) {
const knownError = error as IJsYamlError;
const errorLineNumber = knownError.mark?.line;
const errorMessage = knownError.reason ?? "Unknown error";
const yamlError = error as YAMLParseError;
const errorMessage = yamlError.code ?? "Unknown error";
const errorMarker = {
startLineNumber: errorLineNumber ?? 0,
endLineNumber: errorLineNumber ?? 0,
startColumn: 0,
endColumn: 0,
startLineNumber: yamlError.linePos?.[0].line ?? 0,
endLineNumber: yamlError.linePos?.[1]?.line ?? 0,
startColumn: yamlError.linePos?.[0].col ?? 0,
endColumn: yamlError.linePos?.[1]?.col ?? 0,
severity: 8,
message: errorMessage,
};
model && monacoRef?.current?.editor.setModelMarkers(model, "json", [errorMarker]);

totalErrors.jsYamlError = knownError;
totalErrors.yamlError = yamlError;
}
if (!totalErrors.jsYamlError) {
if (!totalErrors.yamlError) {
customValidate(mainItemsData, serviceItemsData, errorMarkers, totalErrors, configData);
const serverSideErrorElement = findErrorElement(serverSideValidationPath, parsedYamlConfig);
const { line, column } = findLineAndColumn(configData, serverSideErrorElement?.offset);
Expand Down
11 changes: 7 additions & 4 deletions packages/otelbin/src/components/monaco-editor/parseYaml.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// SPDX-FileCopyrightText: 2023 Dash0 Inc.
// SPDX-License-Identifier: Apache-2.0

import { Parser } from "yaml";
import JsYaml, { FAILSAFE_SCHEMA } from "js-yaml";
import YAML, { Parser } from "yaml";
export interface SourceToken {
type:
| "byte-order-mark"
Expand Down Expand Up @@ -240,8 +239,12 @@ export function isOtelColCRD(jsonData: IOtelColCRD) {
}

export function selectConfigType(config: string) {
const jsonData = JsYaml.load(config, { schema: FAILSAFE_SCHEMA }) as any;

let jsonData: any;
try {
jsonData = YAML.parse(config, { logLevel: "error", schema: "failsafe" }) as any;
// Catching and ignoring errors here since validation errors are already displayed in the validation console.
// This prevents additional noise and instability when editing the config.
} catch (e) {}
if (isK8sConfigMap(jsonData)) {
return jsonData.data.relay;
} else if (isOtelColCRD(jsonData)) {
Expand Down
13 changes: 10 additions & 3 deletions packages/otelbin/src/components/react-flow/ReactFlow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React, { type RefObject, useEffect, useMemo } from "react";
import ReactFlow, { Background, Panel, useReactFlow, useNodesState, useEdgesState, useStore } from "reactflow";
import "reactflow/dist/style.css";
import type { IConfig } from "./dataType";
import { Parser } from "yaml";
import YAML, { Parser } from "yaml";
import useEdgeCreator from "./useEdgeCreator";
import { useFocus } from "~/contexts/EditorContext";
import { Minus, Plus, HelpCircle, Lock, Minimize2 } from "lucide-react";
Expand All @@ -22,7 +22,6 @@ import ReceiversNode from "./node-types/ReceiversNode";
import ProcessorsNode from "./node-types/ProcessorsNode";
import { useLayout } from "./layout/useLayout";
import CyclicErrorEdge from "./CyclicErrorEdge";
import JsYaml, { FAILSAFE_SCHEMA } from "js-yaml";

type EditorRefType = RefObject<editor.IStandaloneCodeEditor | null>;

Expand All @@ -40,7 +39,15 @@ export default function Flow({
editorRef: EditorRefType | null;
}) {
const reactFlowInstance = useReactFlow();
const jsonData = useMemo(() => JsYaml.load(value, { schema: FAILSAFE_SCHEMA }) as IConfig, [value]);

const jsonData = useMemo(() => {
try {
return YAML.parse(value, { logLevel: "error", schema: "failsafe" }) as IConfig;
// Catching and ignoring errors here since validation errors are already displayed in the validation console.
// This prevents additional noise and instability when editing the config.
} catch (error: unknown) {}
}, [value]) as IConfig;

const pipelines = useMemo(() => {
const parsedYaml = Array.from(new Parser().parse(value));
const doc = parsedYaml.find((token) => token.type === "document") as Document;
Expand Down

0 comments on commit d491dec

Please sign in to comment.