This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-6800-eae4f218a67978361496075da4cdbea07ee6cd74 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 97f0fdb43fa7e14a501237a9f6662d7dd2ddbf56 Author: Kary Zheng <[email protected]> AuthorDate: Fri Jul 24 14:48:00 2026 -0700 fix(operator): interpolate desc.url so failure message shows the URL (#6800) ### What changes were proposed in this PR? Fixes a Scala string-interpolation bug in `URLFetcherOpExec`'s fetch-failure fallback. The failure branch built its message with `s"Fetch failed for URL: $desc.url"`. In an `s"..."` interpolator, `$desc` expands only the identifier `desc` (the `URLFetcherOpDesc` instance) and `.url` is appended as literal text. Because `LogicalOp` overrides `toString` with `ToStringBuilder.reflectionToString`, the resulting `URL content` cell contained the entire descriptor dump instead of the URL: ``` Fetch failed for URL: org.apache.texera.amber.operator.source.fetcher.URLFetcherOpDesc@5624b9e8[decodingMethod=UTF_8,url=https://this-host-does-not-exist.invalid/x,dummyPropertyList=List(),inputPorts=<null>,operatorId=URLFetcherOpDesc-...,operatorVersion=N/A,outputPorts=<null>].url ``` This both malforms the message and leaks internal operator fields (`operatorId`, `inputPorts`, `dummyPropertyList`, …) into user-facing output. The fix wraps the member access in braces so only `desc.url` is interpolated: ```diff - 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") ``` Now the message reads as intended: ``` Fetch failed for URL: https://this-host-does-not-exist.invalid/x ``` ### Any related issues, documentation, discussions? Closes #6755 ### How was this PR tested? Added an automated regression test to `URLFetcherOpExecSpec` that exercises the fetch-failure branch **offline and deterministically**: pointing the operator at a `file://` URL for a nonexistent path makes `URLFetchUtil.getInputStreamFromURL` return `None` with no network dependency. The test asserts the fallback cell is exactly `Fetch failed for URL: <url>` and does **not** contain the descriptor dump (`URLFetcherOpDesc`, `operatorId`, …), so it fails on the pre-fix `$desc.url` behavior and passes with the fix. ``` sbt "WorkflowOperator/testOnly org.apache.texera.amber.operator.source.fetcher.URLFetcherOpExecSpec" ... Tests: succeeded 3, failed 0, canceled 0, ignored 0, pending 0 All tests passed. ``` Also reproduced end-to-end before the fix by pointing a **URL Fetcher** operator at an unreachable address (`https://this-host-does-not-exist.invalid/x`) and inspecting the `URL content` output cell: it showed the full `URLFetcherOpDesc[...]` dump followed by `.url`; after the fix it shows `Fetch failed for URL: https://this-host-does-not-exist.invalid/x`. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 4.8) --------- Signed-off-by: Kary Zheng <[email protected]> Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]> Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Xinyuan Lin <[email protected]> Co-authored-by: Yicong Huang <[email protected]> --- .../operator/source/fetcher/URLFetcherOpExec.scala | 8 +++----- .../source/fetcher/URLFetcherOpExecSpec.scala | 23 ++++++++++++++++++++++ 2 files changed, 26 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 3f7b45421a..e6035acb10 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 { 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 47770cbae8..a1a3967d65 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,27 @@ 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 { + 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] + + 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")) + } + }
