Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MRESOLVER-335] Proposal for better errors #386

Merged
merged 6 commits into from
Dec 11, 2023
Merged

[MRESOLVER-335] Proposal for better errors #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
969dbd8
[MRESOLVER-335] Proposal for better errors
cstamas Dec 4, 2023
9746e49
Merge remote-tracking branch 'origin/master' into MRESOLVER-335
cstamas Dec 5, 2023
1cecc04
Hide system repository
cstamas Dec 5, 2023
79976cf
Simplify
cstamas Dec 5, 2023
07083ff
Merge remote-tracking branch 'origin/master' into MRESOLVER-335
cstamas Dec 9, 2023
0015248
Simplify
cstamas Dec 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 29 additions & 32 deletions ...resolver-api/src/main/java/org/eclipse/aether/resolution/ArtifactResolutionException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@

import java.util.Collections;
import java.util.List;
import java.util.Map;

import org.eclipse.aether.RepositoryException;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.repository.LocalArtifactResult;
import org.eclipse.aether.transfer.ArtifactFilteredOutException;
import org.eclipse.aether.transfer.ArtifactNotFoundException;
import org.eclipse.aether.transfer.RepositoryOfflineException;

import static java.util.stream.Collectors.joining;

/**
* Thrown in case of a unresolvable artifacts.
*/
public class ArtifactResolutionException extends RepositoryException {

private final transient List<ArtifactResult> results;

/**
Expand All @@ -40,8 +40,8 @@ public class ArtifactResolutionException extends RepositoryException {
* @param results The resolution results at the point the exception occurred, may be {@code null}.
*/
public ArtifactResolutionException(List<ArtifactResult> results) {
super(getMessage(results), getCause(results));
this.results = (results != null) ? results : Collections.<ArtifactResult>emptyList();
super(getMessage(results));
this.results = results != null ? results : Collections.emptyList();
}

/**
Expand All @@ -51,8 +51,8 @@ public ArtifactResolutionException(List<ArtifactResult> results) {
* @param message The detail message, may be {@code null}.
*/
public ArtifactResolutionException(List<ArtifactResult> results, String message) {
super(message, getCause(results));
this.results = (results != null) ? results : Collections.<ArtifactResult>emptyList();
super(message);
this.results = results != null ? results : Collections.emptyList();
}

/**
Expand All @@ -64,14 +64,14 @@ public ArtifactResolutionException(List<ArtifactResult> results, String message)
*/
public ArtifactResolutionException(List<ArtifactResult> results, String message, Throwable cause) {
super(message, cause);
this.results = (results != null) ? results : Collections.<ArtifactResult>emptyList();
this.results = results != null ? results : Collections.emptyList();
}

/**
* Gets the resolution results at the point the exception occurred. Despite being incomplete, callers might want to
* use these results to fail gracefully and continue their operation with whatever interim data has been gathered.
*
* @return The resolution results or {@code null} if unknown.
* @return The resolution results, never {@code null} (empty if unknown).
*/
public List<ArtifactResult> getResults() {
return results;
Expand All @@ -88,6 +88,9 @@ public ArtifactResult getResult() {
}

private static String getMessage(List<? extends ArtifactResult> results) {
if (results == null) {
return null;
}
StringBuilder buffer = new StringBuilder(256);

buffer.append("The following artifacts could not be resolved: ");
Expand All @@ -114,36 +117,30 @@ private static String getMessage(List<? extends ArtifactResult> results) {
}
}

Throwable cause = getCause(results);
String cause = getCause(results);
if (cause != null) {
buffer.append(": ").append(cause.getMessage());
buffer.append(": ").append(cause);
}

return buffer.toString();
}

private static Throwable getCause(List<? extends ArtifactResult> results) {
private static String getCause(List<? extends ArtifactResult> results) {
for (ArtifactResult result : results) {
if (!result.isResolved()) {
Throwable notFound = null, offline = null;
for (Throwable t : result.getExceptions()) {
if (t instanceof ArtifactNotFoundException) {
if (notFound == null || notFound instanceof ArtifactFilteredOutException) {
notFound = t;
}
if (offline == null && t.getCause() instanceof RepositoryOfflineException) {
offline = t;
}
} else {
return t;
}
}
if (offline != null) {
return offline;
}
if (notFound != null) {
return notFound;
Map<ArtifactRepository, List<Exception>> exMap = result.getMappedExceptions();
if (!result.isResolved() && !exMap.isEmpty()) {
StringBuilder stringBuilder = new StringBuilder();
for (Map.Entry<ArtifactRepository, List<Exception>> t : exMap.entrySet()) {
stringBuilder
.append("[")
.append(t.getKey().getId())
.append(" repository ")
.append(t.getValue().stream()
.map(Exception::getMessage)
.collect(joining(",")))
.append("]");
}
return stringBuilder.toString();
}
}
return null;
Expand Down
74 changes: 64 additions & 10 deletions maven-resolver-api/src/main/java/org/eclipse/aether/resolution/ArtifactResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
*/
package org.eclipse.aether.resolution;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CopyOnWriteArrayList;

import org.eclipse.aether.RepositorySystem;
import org.eclipse.aether.artifact.Artifact;
Expand All @@ -38,9 +38,34 @@
*/
public final class ArtifactResult {

/**
* A sentinel object, that is used as key for exceptions that had no related repository during resolution.
*
* @since 2.0.0
*/
public static final ArtifactRepository NO_REPOSITORY = new NoRepository();

private static final class NoRepository implements ArtifactRepository {

private NoRepository() {}

public String getContentType() {
return "unknown";
}

public String getId() {
return "unknown";
}

@Override
public String toString() {
return getId();
}
}

private final ArtifactRequest request;

private List<Exception> exceptions;
private final Map<ArtifactRepository, List<Exception>> exceptions;

private Artifact artifact;

Expand All @@ -55,7 +80,7 @@ public final class ArtifactResult {
*/
public ArtifactResult(ArtifactRequest request) {
this.request = requireNonNull(request, "artifact request cannot be null");
exceptions = Collections.emptyList();
this.exceptions = new ConcurrentHashMap<>();
}

/**
Expand Down Expand Up @@ -95,8 +120,25 @@ public ArtifactResult setArtifact(Artifact artifact) {
*
* @return The exceptions that occurred, never {@code null}.
* @see #isResolved()
* @see #isMissing()
*/
public List<Exception> getExceptions() {
ArrayList<Exception> result = new ArrayList<>();
exceptions.values().forEach(result::addAll);
return result;
}

/**
* Gets the exceptions that occurred while resolving the artifact. Note that this map can be non-empty even if the
* artifact was successfully resolved, e.g. when one of the contacted remote repositories didn't contain the
* artifact but a later repository eventually contained it.
*
* @return Map of exceptions per repository, that occurred during resolution, never {@code null}.
* @see #isResolved()
* @see #isMissing()
* @since 2.0.0
*/
public Map<ArtifactRepository, List<Exception>> getMappedExceptions() {
return exceptions;
}

Expand All @@ -105,13 +147,25 @@ public List<Exception> getExceptions() {
*
* @param exception The exception to record, may be {@code null}.
* @return This result for chaining, never {@code null}.
* @deprecated Use {@link #addException(ArtifactRepository, Exception)} method instead.
*/
@Deprecated
public ArtifactResult addException(Exception exception) {
if (exception != null) {
if (exceptions.isEmpty()) {
exceptions = new ArrayList<>();
}
exceptions.add(exception);
return addException(NO_REPOSITORY, exception);
}

/**
* Records the specified exception while resolving the artifact.
*
* @param exception The exception to record, may be {@code null}.
* @return This result for chaining, never {@code null}.
* @since 2.0.0
*/
public ArtifactResult addException(ArtifactRepository repository, Exception exception) {
if (repository != null && exception != null) {
exceptions
.computeIfAbsent(repository, k -> new CopyOnWriteArrayList<>())
.add(exception);
}
return this;
}
Expand Down
41 changes: 20 additions & 21 deletions ...resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,7 @@
import org.eclipse.aether.impl.UpdateCheck;
import org.eclipse.aether.impl.UpdateCheckManager;
import org.eclipse.aether.impl.VersionResolver;
import org.eclipse.aether.repository.ArtifactRepository;
import org.eclipse.aether.repository.LocalArtifactRegistration;
import org.eclipse.aether.repository.LocalArtifactRequest;
import org.eclipse.aether.repository.LocalArtifactResult;
import org.eclipse.aether.repository.LocalRepository;
import org.eclipse.aether.repository.LocalRepositoryManager;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.repository.RepositoryPolicy;
import org.eclipse.aether.repository.WorkspaceReader;
import org.eclipse.aether.repository.*;
import org.eclipse.aether.resolution.ArtifactRequest;
import org.eclipse.aether.resolution.ArtifactResolutionException;
import org.eclipse.aether.resolution.ArtifactResult;
Expand Down Expand Up @@ -239,7 +231,8 @@ private List<ArtifactResult> resolve(
File file = new File(localPath);
if (!file.isFile()) {
failures = true;
result.addException(new ArtifactNotFoundException(artifact, null));
result.addException(
ArtifactResult.NO_REPOSITORY, new ArtifactNotFoundException(artifact, null));
} else {
artifact = artifact.setFile(file);
result.setArtifact(artifact);
Expand All @@ -254,8 +247,10 @@ private List<ArtifactResult> resolve(
for (RemoteRepository repository : remoteRepositories) {
RemoteRepositoryFilter.Result filterResult = filter.acceptArtifact(repository, artifact);
if (!filterResult.isAccepted()) {
result.addException(new ArtifactFilteredOutException(
artifact, repository, filterResult.reasoning()));
result.addException(
repository,
new ArtifactFilteredOutException(
artifact, repository, filterResult.reasoning()));
filteredRemoteRepositories.remove(repository);
}
}
Expand All @@ -268,7 +263,11 @@ private List<ArtifactResult> resolve(
versionRequest.setTrace(trace);
versionResult = versionResolver.resolveVersion(session, versionRequest);
} catch (VersionResolutionException e) {
result.addException(e);
if (filteredRemoteRepositories.isEmpty()) {
result.addException(lrm.getRepository(), e);
} else {
filteredRemoteRepositories.forEach(r -> result.addException(r, e));
}
continue;
}

Expand Down Expand Up @@ -315,7 +314,7 @@ private List<ArtifactResult> resolve(
result.setArtifact(artifact);
artifactResolved(session, trace, artifact, result.getRepository(), null);
} catch (ArtifactTransferException e) {
result.addException(e);
result.addException(lrm.getRepository(), e);
}
if (filter == null && simpleLrmInterop && !local.isAvailable()) {
/*
Expand Down Expand Up @@ -355,7 +354,7 @@ private List<ArtifactResult> resolve(
+ repo.getUrl() + ") in offline mode and the artifact " + artifact
+ " has not been downloaded from it before.",
e);
result.addException(exception);
result.addException(repo, exception);
continue;
}

Expand Down Expand Up @@ -399,7 +398,7 @@ private List<ArtifactResult> resolve(
failures = true;
if (result.getExceptions().isEmpty()) {
Exception exception = new ArtifactNotFoundException(request.getArtifact(), null);
result.addException(exception);
result.addException(result.getRepository(), exception);
}
RequestTrace trace = RequestTrace.newChild(request.getTrace(), request);
artifactResolved(session, trace, request.getArtifact(), null, result.getExceptions());
Expand Down Expand Up @@ -524,7 +523,7 @@ private List<ArtifactDownload> gatherDownloads(RepositorySystemSession session,
item.updateCheck = check;
updateCheckManager.checkArtifact(session, check);
if (!check.isRequired()) {
item.result.addException(check.getException());
item.result.addException(group.repository, check.getException());
continue;
}
}
Expand Down Expand Up @@ -560,10 +559,10 @@ private void evaluateDownloads(RepositorySystemSession session, ResolutionGroup
new LocalArtifactRegistration(artifact, group.repository, download.getSupportedContexts()));
} catch (ArtifactTransferException e) {
download.setException(e);
item.result.addException(e);
item.result.addException(group.repository, e);
}
} else {
item.result.addException(download.getException());
item.result.addException(group.repository, download.getException());
}

/*
Expand Down Expand Up @@ -595,12 +594,12 @@ private void artifactResolved(
RequestTrace trace,
Artifact artifact,
ArtifactRepository repository,
List<Exception> exceptions) {
Collection<Exception> exceptions) {
RepositoryEvent.Builder event = new RepositoryEvent.Builder(session, EventType.ARTIFACT_RESOLVED);
event.setTrace(trace);
event.setArtifact(artifact);
event.setRepository(repository);
event.setExceptions(exceptions);
event.setExceptions(exceptions != null ? new ArrayList<>(exceptions) : null);
if (artifact != null) {
event.setFile(artifact.getFile());
}
Expand Down
Loading