Skip to content

Commit

Permalink
fix(yarn): Fix up the error handling in getRemotePackageDetails()
Browse files Browse the repository at this point in the history
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]: ad9a363

Signed-off-by: Frank Viernau <[email protected]>
  • Loading branch information
fviernau committed Oct 24, 2024
1 parent 9d5c10b commit f6f9054
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 10 deletions.
19 changes: 13 additions & 6 deletions plugins/package-managers/node/src/main/kotlin/Yarn.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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<JsonElement> =
runCatching {
Expand Down
8 changes: 4 additions & 4 deletions plugins/package-managers/node/src/test/kotlin/YarnTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand All @@ -44,21 +44,21 @@ 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"
}

"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"
}
Expand Down

0 comments on commit f6f9054

Please sign in to comment.