RohanExploit commented on PR #11823: URL: https://github.com/apache/seatunnel/pull/11823#issuecomment-5339972863
@DanielLeens thanks for the detailed review. Issue 1 was a real scope miss on my part, not a deliberate narrowing. The linkage failure happens while `FileSystem.get()` builds the `DFSClient` and `FsTracer`, which has nothing to do with Kerberos, so wrapping only that branch left the other two call sites just as opaque as before. I went with Option A. `initialize()` and `initializeWithRemoteUserLogin()` now both go through `getFileSystemWithDiagnostics()`. I checked the propagation instead of assuming it: `initialize()` is `@SneakyThrows` and `HadoopLoginFactory.LoginFunction#run` declares `throws Exception`, so both were a straight swap. Grepping for `FileSystem.get(configuration)` in the file now returns one hit, the call inside the helper. Issue 2 is fixed by widening the catch to `LinkageError`. `NoSuchFieldError` and `IncompatibleClassChangeError` come out of the same jar mismatch, and `LinkageError` still leaves `OutOfMemoryError` and `StackOverflowError` alone. The `IOException` message body is unchanged. Javadoc is on both `getFileSystemWithDiagnostics` and `wrapClasspathMismatch` now, replacing the old block comment. It says the failure is not specific to any one authentication mode and that all three call sites route through the helper, and it refers to `LinkageError` rather than `NoSuchMethodError`. New tests: * `testWrapClasspathMismatchRewritesNoClassDefFoundError` * `testWrapClasspathMismatchRewritesNoSuchFieldError` * `testWrapClasspathMismatchRewritesIncompatibleClassChangeError` * `testWrapClasspathMismatchDoesNotSwallowUnrelatedErrors`, which throws an `OutOfMemoryError` and asserts the same instance comes back out unwrapped. That one is what makes the widening to `LinkageError` reviewable. * `testWrapClasspathMismatchPropagatesIOExceptionUnchanged`, where a plain `IOException` from the supplier comes back identical with a null cause. The four rewrite tests share a private `assertRewrittenAsDiagnosticIOException(LinkageError)` helper that asserts the original is the cause and the message names the Hadoop client version mismatch. The existing `NoSuchMethodError` test uses it too. All plain `@Test`, no I/O, no sleeps, no new dependencies. I sanity checked them by putting the old two type catch back: `NoSuchFieldError` and `IncompatibleClassChangeError` fail, everything else passes. You were right that the title was narrower than the mechanism. Retitled it and rewrote the PR body around all three call sites. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
