sk0x50 commented on code in PR #2225: URL: https://github.com/apache/ignite-3/pull/2225#discussion_r1236972592
########## modules/api/src/main/java/org/apache/ignite/lang/IgniteExceptionMapperUtil.java: ########## @@ -0,0 +1,149 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.lang; + +import static java.util.Collections.unmodifiableMap; +import static org.apache.ignite.lang.ErrorGroups.Common.INTERNAL_ERR; + +import java.util.HashMap; +import java.util.Map; +import java.util.ServiceLoader; + +/** + * This utility class provides an ability to map Ignite internal exceptions to Ignite public ones. + */ +public class IgniteExceptionMapperUtil { + /** All exception mappers to be used to map internal exceptions to public ones. */ + private static final Map<Class<? extends Exception>, IgniteExceptionMapper<? extends Exception, ? extends Exception>> + EXCEPTION_CONVERTERS; + + static { + Map<Class<? extends Exception>, IgniteExceptionMapper<? extends Exception, ? extends Exception>> mappers = new HashMap<>(); + + ServiceLoader + .load(IgniteExceptionMappersProvider.class) + .forEach(provider -> provider.mappers().forEach(m -> registerMapping(m, mappers))); + + EXCEPTION_CONVERTERS = unmodifiableMap(mappers); + } + + /** + * Add a new mapping to already registered ones. + * + * @param mapper Exception mapper from internal exception to a public one. + * @param registeredMappings Already registered mappings. + * @throws IgniteException If a mapper for the given {@code clazz} already registered, + * or {@code clazz} represents Java standard exception like {@link NullPointerException}, {@link IllegalArgumentException}. + */ + static void registerMapping( + IgniteExceptionMapper<? extends Exception, ? extends Exception> mapper, + Map<Class<? extends Exception>, IgniteExceptionMapper<? extends Exception, ? extends Exception>> registeredMappings) { + if (registeredMappings.containsKey(mapper.mappingFrom())) { + throw new IgniteException( + INTERNAL_ERR, + "Failed to register exception mapper, duplicate found [class=" + mapper.mappingFrom().getCanonicalName() + ']'); + } + + if (isJavaStandardException(mapper.mappingFrom())) { + throw new IgniteException( + INTERNAL_ERR, + "Failed to register exception mapper. " + + "Mapping for this class is prohibited [class=" + mapper.mappingFrom().getCanonicalName() + ']'); + } + + registeredMappings.put(mapper.mappingFrom(), mapper); + } + + /** + * This method provides a mapping from internal exception to Ignite public ones. + * + * <p>The rules of mapping are the following:</p> + * <ul> + * <li>any instance of {@link Error} is returned as is, except {@link AssertionError} + * that always be mapped to {@link IgniteException} with the {@link ErrorGroups.Common#INTERNAL_ERR} error code.</li> + * <li>any instance of Java standard exception like {@link NullPointerException} is returned as is.</li> + * <li>any instance of {@link IgniteException} or {@link IgniteCheckedException} is returned as is.</li> + * <li>if there are no any mappers that can do a mapping from the given error to a public exception, + * then {@link IgniteException} with the {@link ErrorGroups.Common#INTERNAL_ERR} error code is returned.</li> + * </ul> + * + * @param origin Exception to be mapped. + * @return Public exception. + */ + public static Throwable mapToPublicException(Throwable origin) { + if (origin instanceof Error) { + if (origin instanceof AssertionError) { + return new IgniteException(INTERNAL_ERR, origin); + } + + return origin; + } + + if (origin instanceof IgniteException || origin instanceof IgniteCheckedException) { + return origin; + } + + if (isJavaStandardException(origin)) { + return origin; + } + + IgniteExceptionMapper<? extends Exception, ? extends Exception> m = EXCEPTION_CONVERTERS.get(origin.getClass()); + if (m != null) { + Exception mapped = map(m, origin); + + assert mapped instanceof IgniteException || mapped instanceof IgniteCheckedException : + "Unexpected mapping of internal exception to a public one [origin=" + origin + ", mapped=" + mapped + ']'; + + return mapped; + } + + // There are no exception mappings for the given exception. This case should be considered as internal error. + return new IgniteException(INTERNAL_ERR, origin); Review Comment: I don't think so. Let's assume that we have hundreds of codes (it is an absolutely possible case). Should we create hundreds of exception classes? IMHO, it is OK to map several error codes to one exception class, assuming that these error codes relate to the one error group of course. In this particular case, we can introduce a new type with a proper name, I think. -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
