From f6f90549004ef54ce918580e8c18a54f19f31920 Mon Sep 17 00:00:00 2001 From: Frank Viernau Date: Thu, 24 Oct 2024 10:27:28 +0200 Subject: [PATCH] fix(yarn): Fix up the error handling in `getRemotePackageDetails()` Previously, calling `parseYarnInfo(process.stderr)` was useless, because it attempts to only parse a `PackageJson` instance from `stderr` which always fails, because `yarn` does not output such data to `stderr`. However, in [1] the logging of errors obtained from `stderr` got (partially) removed. Fix this up by logging any errors found in `stderr` in case no `PackageJson` instance could be parsed from `stdout`. [1]: ad9a3639604aacfa6aa3fd831593201f42e78483 Signed-off-by: Frank Viernau --- .../node/src/main/kotlin/Yarn.kt | 19 +++++++++++++------ .../node/src/test/kotlin/YarnTest.kt | 8 ++++---- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/plugins/package-managers/node/src/main/kotlin/Yarn.kt b/plugins/package-managers/node/src/main/kotlin/Yarn.kt index 481668e37087..d47323a23704 100644 --- a/plugins/package-managers/node/src/main/kotlin/Yarn.kt +++ b/plugins/package-managers/node/src/main/kotlin/Yarn.kt @@ -20,6 +20,7 @@ package org.ossreviewtoolkit.plugins.packagemanagers.node import java.io.File +import java.lang.invoke.MethodHandles import kotlin.time.Duration.Companion.days @@ -29,8 +30,9 @@ import kotlinx.serialization.json.JsonElement import kotlinx.serialization.json.JsonObject import kotlinx.serialization.json.JsonPrimitive import kotlinx.serialization.json.decodeToSequence +import kotlinx.serialization.json.jsonPrimitive -import org.apache.logging.log4j.kotlin.logger +import org.apache.logging.log4j.kotlin.loggerOf import org.ossreviewtoolkit.analyzer.AbstractPackageManagerFactory import org.ossreviewtoolkit.model.config.AnalyzerConfiguration @@ -39,6 +41,7 @@ import org.ossreviewtoolkit.plugins.packagemanagers.node.utils.NodePackageManage import org.ossreviewtoolkit.plugins.packagemanagers.node.utils.NpmDetection import org.ossreviewtoolkit.utils.common.DiskCache import org.ossreviewtoolkit.utils.common.Os +import org.ossreviewtoolkit.utils.common.alsoIfNull import org.ossreviewtoolkit.utils.common.mebibytes import org.ossreviewtoolkit.utils.ort.ortDataDirectory @@ -92,14 +95,14 @@ class Yarn( val process = run(workingDir, "info", "--json", packageName) - return parseYarnInfo(process.stdout)?.also { + return parseYarnInfo(process.stdout, process.stderr)?.also { yarnInfoCache.write(packageName, Json.encodeToString(it)) - } ?: checkNotNull(parseYarnInfo(process.stderr)).also { - logger.warn { "Error running '${process.commandLine}' in directory $workingDir: $it" } } } } +private val logger = loggerOf(MethodHandles.lookup().lookupClass()) + /** * Parse the given [stdout] of a Yarn _info_ command to a [PackageJson]. The output is typically a JSON object with the * metadata of the package that was queried. However, under certain circumstances, Yarn may return multiple JSON objects @@ -110,8 +113,12 @@ class Yarn( * Note: The mentioned network issue can be reproduced by setting the network timeout to be very short via the command * line option '--network-timeout'. */ -internal fun parseYarnInfo(stdout: String): PackageJson? = - extractDataNodes(stdout, "inspect").firstOrNull()?.let(::parsePackageJson) +internal fun parseYarnInfo(stdout: String, stderr: String): PackageJson? = + extractDataNodes(stdout, "inspect").firstOrNull()?.let(::parsePackageJson).alsoIfNull { + extractDataNodes(stderr, "error").forEach { + logger.warn { "Error parsing Yarn info: ${it.jsonPrimitive.content}" } + } + } private fun extractDataNodes(output: String, type: String): Set = runCatching { diff --git a/plugins/package-managers/node/src/test/kotlin/YarnTest.kt b/plugins/package-managers/node/src/test/kotlin/YarnTest.kt index 42932d24c98b..defcfd72d1a4 100644 --- a/plugins/package-managers/node/src/test/kotlin/YarnTest.kt +++ b/plugins/package-managers/node/src/test/kotlin/YarnTest.kt @@ -33,7 +33,7 @@ class YarnTest : WordSpec({ "parse a valid JSON string" { val json = fileWithRetries.readLines()[3] - val packageJson = parseYarnInfo(json) + val packageJson = parseYarnInfo(json, "") packageJson?.homepage shouldBe "https://github.com/watson/bonjour/local" } @@ -44,13 +44,13 @@ class YarnTest : WordSpec({ Also not on any line. """.trimIndent() - parseYarnInfo(json) should beNull() + parseYarnInfo(json, "") should beNull() } "parse a JSON string with multiple objects" { val json = fileWithRetries.readText() - val packageJson = parseYarnInfo(json) + val packageJson = parseYarnInfo(json, "") packageJson?.homepage shouldBe "https://github.com/watson/bonjour/local" } @@ -58,7 +58,7 @@ class YarnTest : WordSpec({ "handle a type property that is not a primitive" { val json = File("src/test/assets/yarn-package-data-with-wrong-type.txt").readText() - val packageJson = parseYarnInfo(json) + val packageJson = parseYarnInfo(json, "") packageJson?.homepage shouldBe "https://github.com/watson/bonjour/local" }