From ba001b4af7469c59f27e91d36577396340d9ec0e Mon Sep 17 00:00:00 2001 From: kary zheng Date: Wed, 22 Jul 2026 13:54:30 -0700 Subject: [PATCH 1/4] fix(URLFetcher): interpolate desc.url so failure message shows the URL In URLFetcherOpExec, the fetch-failure fallback used `s"...$desc.url"`, which expands only `$desc` (the URLFetcherOpDesc instance) and appends a literal `.url`. Because LogicalOp overrides toString with ToStringBuilder.reflectionToString, the "URL content" cell got the whole descriptor dump (operatorId, inputPorts, dummyPropertyList, ...) instead of the URL. Wrap the member access in braces (`${desc.url}`) so the message reads `Fetch failed for URL: ` as intended and stops leaking internal operator fields into user-facing output. Closes #6755 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../texera/amber/operator/source/fetcher/URLFetcherOpExec.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala index 3f7b45421a2..b29c58ab4d3 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala @@ -35,7 +35,7 @@ class URLFetcherOpExec(descString: String) extends SourceOperatorExecutor { val input = getInputStreamFromURL(urlObj) val contentInputStream = input match { case Some(value) => value - case None => IOUtils.toInputStream(s"Fetch failed for URL: $desc.url", "UTF-8") + case None => IOUtils.toInputStream(s"Fetch failed for URL: ${desc.url}", "UTF-8") } Iterator(if (desc.decodingMethod == DecodingMethod.UTF_8) { TupleLike(IOUtils.toString(contentInputStream, "UTF-8")) From 86cd1c4bf0359d030308475752c5f11b10f2cc76 Mon Sep 17 00:00:00 2001 From: kary zheng Date: Wed, 22 Jul 2026 15:20:26 -0700 Subject: [PATCH 2/4] test(URLFetcher): regression test for the #6755 fetch-failure message Assert the fetch-failure fallback interpolates `desc.url` itself, not the URLFetcherOpDesc reflectionToString dump. Uses a file:// URL to a nonexistent path so getInputStreamFromURL returns None deterministically and offline (no network), exercising the failure branch without external connectivity. Verified: passes with the ${desc.url} fix, fails with the pre-fix $desc.url. Co-Authored-By: Claude Opus 4.8 --- .../source/fetcher/URLFetcherOpExecSpec.scala | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala index 47770cbae84..7b6204eb939 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala @@ -44,4 +44,21 @@ class URLFetcherOpExecSpec extends AnyFlatSpec with BeforeAndAfter { assert(!iterator.hasNext) } + // On a failed fetch the fallback message must interpolate `desc.url` itself, + // not the descriptor's reflectionToString dump. A file:// URL to a nonexistent + // path makes getInputStreamFromURL return None deterministically and offline, + // so the failure branch is exercised without depending on external connectivity. + it should "report only the URL, not the operator descriptor, when the fetch fails" in { + opDesc.url = "file:///nonexistent/texera-urlfetcher-regression" + opDesc.decodingMethod = DecodingMethod.UTF_8 + val fetcherOpExec = new URLFetcherOpExec(objectMapper.writeValueAsString(opDesc)) + val content = fetcherOpExec.produceTuple().next().getFields.toList.head.asInstanceOf[String] + + assert(content == s"Fetch failed for URL: ${opDesc.url}") + // Guard against the pre-fix `$desc.url` behavior, which leaked the whole + // descriptor dump (class name + internal fields) into the message. + assert(!content.contains("URLFetcherOpDesc")) + assert(!content.contains("operatorId")) + } + } From 6ef2f3c077ad752df307bc021f52e2b712420c59 Mon Sep 17 00:00:00 2001 From: Kary Zheng <150742834+kz930@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:34:11 -0700 Subject: [PATCH 3/4] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Kary Zheng <150742834+kz930@users.noreply.github.com> --- .../operator/source/fetcher/URLFetcherOpExecSpec.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala index 7b6204eb939..a1a3967d65c 100644 --- a/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala +++ b/common/workflow-operator/src/test/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExecSpec.scala @@ -49,7 +49,13 @@ class URLFetcherOpExecSpec extends AnyFlatSpec with BeforeAndAfter { // path makes getInputStreamFromURL return None deterministically and offline, // so the failure branch is exercised without depending on external connectivity. it should "report only the URL, not the operator descriptor, when the fetch fails" in { - opDesc.url = "file:///nonexistent/texera-urlfetcher-regression" + val missingUrl = + java.nio.file.Files + .createTempDirectory("texera-urlfetcher-regression-") + .resolve("missing") + .toUri + .toString + opDesc.url = missingUrl opDesc.decodingMethod = DecodingMethod.UTF_8 val fetcherOpExec = new URLFetcherOpExec(objectMapper.writeValueAsString(opDesc)) val content = fetcherOpExec.produceTuple().next().getFields.toList.head.asInstanceOf[String] From 855ddafb369bd1b4b09285c002256909d1911d1e Mon Sep 17 00:00:00 2001 From: Yicong Huang <17627829+Yicong-Huang@users.noreply.github.com> Date: Fri, 24 Jul 2026 14:00:15 -0700 Subject: [PATCH 4/4] refactor(URLFetcher): use getOrElse so the fallback line is fully branch-covered JaCoCo marks the `case None` line of the Option match as partially covered because the match carries an unreachable MatchError branch, which makes Codecov count the patched line as uncovered (patch 0%). getOrElse expresses the same fallback without a pattern match, so the line is fully covered by the existing regression test. Co-Authored-By: Claude Fable 5 --- .../amber/operator/source/fetcher/URLFetcherOpExec.scala | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala index b29c58ab4d3..e6035acb102 100644 --- a/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala +++ b/common/workflow-operator/src/main/scala/org/apache/texera/amber/operator/source/fetcher/URLFetcherOpExec.scala @@ -32,11 +32,9 @@ class URLFetcherOpExec(descString: String) extends SourceOperatorExecutor { override def produceTuple(): Iterator[TupleLike] = { val urlObj = new URL(desc.url) - val input = getInputStreamFromURL(urlObj) - val contentInputStream = input match { - case Some(value) => value - case None => IOUtils.toInputStream(s"Fetch failed for URL: ${desc.url}", "UTF-8") - } + val contentInputStream = getInputStreamFromURL(urlObj).getOrElse( + IOUtils.toInputStream(s"Fetch failed for URL: ${desc.url}", "UTF-8") + ) Iterator(if (desc.decodingMethod == DecodingMethod.UTF_8) { TupleLike(IOUtils.toString(contentInputStream, "UTF-8")) } else {