steveloughran commented on code in PR #6884:
URL: https://github.com/apache/hadoop/pull/6884#discussion_r1832652535
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/ErrorTranslation.java:
##########
@@ -105,6 +112,54 @@ private static Throwable getInnermostThrowable(Throwable
thrown, Throwable outer
return getInnermostThrowable(thrown.getCause(), thrown);
}
+ /**
+ * Attempts to extract the underlying SdkException from an S3 encryption
client exception.
+ *
+ * <p>This method is designed to handle exceptions that may be wrapped within
+ * S3EncryptionClientExceptions. It performs the following steps:
+ * <ol>
+ * <li>Checks if the input exception is null.</li>
+ * <li>Verifies if the exception contains the S3EncryptionClientException
signature.</li>
+ * <li>Examines the cause chain to find the most relevant
SdkException.</li>
+ * </ol>
+ *
+ * <p>The method aims to unwrap nested exceptions to provide more meaningful
+ * error information, particularly in the context of S3 encryption
operations.
+ *
+ * @param exception The SdkException to analyze. This may be a wrapper
exception
+ * containing a more specific underlying cause.
+ * @return The extracted SdkException if found within the exception chain,
+ * or the original exception if no relevant nested exception is
found.
+ * Returns null if the input exception is null.
+ *
+ * @see SdkException
+ * @see AwsServiceException
+ */
+ public static SdkException
maybeExtractSdkExceptionFromEncryptionClientException(
Review Comment:
this is a bit long. How about `maybeProcessEncryptionClientException()`
this says it is handled, without saying exactly what happens.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/EncryptionS3ClientFactory.java:
##########
@@ -260,21 +274,32 @@ private S3AsyncClient
createS3AsyncEncryptionClient(S3ClientCreationParameters p
return s3EncryptionAsyncClientBuilder.build();
}
-
/**
- * Retrieves an instance of the Keyring provider based on the provided class
name.
+ * Creates and returns a Keyring provider instance based on the given class
name.
+ *
+ * <p>This method attempts to instantiate a Keyring provider using
reflection. It first tries
+ * to create an instance using the standard ReflectionUtils.newInstance
method. If that fails,
+ * it falls back to an alternative instantiation method, which is primarily
used for testing
+ * purposes (specifically for CustomKeyring.java).
*
- * @param className The fully qualified class name of the Keyring provider
implementation.
- * @param conf The Configuration object containing the necessary
configuration properties.
- * @return An instance of the Keyring provider.
+ * @param className The fully qualified class name of the Keyring provider
to instantiate.
+ * @param conf The Configuration object to be passed to the Keyring provider
constructor.
+ * @return An instance of the specified Keyring provider.
+ * @throws RuntimeException If unable to create the Keyring provider
instance.
*/
- private Keyring getKeyringProvider(String className, Configuration conf) {
+ private Keyring getKeyringProvider(String className, Configuration conf) {
+ Class<? extends Keyring> keyringProviderClass =
getCustomKeyringProviderClass(className);
try {
- return
ReflectionUtils.newInstance(getCustomKeyringProviderClass(className), conf);
+ return ReflectionUtils.newInstance(keyringProviderClass, conf);
} catch (Exception e) {
- // this is for testing purpose to support CustomKeyring.java
- return
ReflectionUtils.newInstance(getCustomKeyringProviderClass(className), conf,
- new Class[] {Configuration.class}, conf);
+ LOG.warn("Failed to create Keyring provider", e);
+ // This is for testing purposes to support CustomKeyring.java
+ try {
+ return ReflectionUtils.newInstance(keyringProviderClass, conf,
+ new Class[] {Configuration.class}, conf);
+ } catch (Exception ex) {
+ throw new RuntimeException("Failed to create Keyring provider", ex);
Review Comment:
same comment about including the wrapped exception text
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/EncryptionS3ClientFactory.java:
##########
@@ -246,8 +255,13 @@ private S3AsyncClient
createS3AsyncEncryptionClient(S3ClientCreationParameters p
s3EncryptionAsyncClientBuilder.cryptoMaterialsManager(kmsCryptoMaterialsManager);
break;
case CUSTOM:
- Keyring keyring =
getKeyringProvider(cseMaterials.getCustomKeyringClassName(),
- cseMaterials.getConf());
+ Keyring keyring;
+ try {
+ keyring =
+ getKeyringProvider(cseMaterials.getCustomKeyringClassName(),
cseMaterials.getConf());
+ } catch (RuntimeException e) {
+ throw new IOException("Failed to instantiate a custom keyring
provider", e);
Review Comment:
can you include the error text of e in the new IOE; it's really easy to lose
the base exception in log stack chains with deep wrapping/rewrapping.
{code}
IOException("Failed to instantiate a custom keyring provider: " + e, e)
{code}
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
