Skip to content

Commit

Permalink
Do not hide init errors in BundleComponent
Browse files Browse the repository at this point in the history
Currently BundleComponent do not propagate errors but only log them,
this can lead to a situation where it is unclear what calls fails.

This changes the init method to throw an CoreException in all cases
where it encounters a problem and instead use dedicated catch whenever
it seems suitable.

See #553
  • Loading branch information
laeubi committed Oct 8, 2023
1 parent 540c9ef commit 17ccaed
Showing 1 changed file with 36 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ public class BundleComponent extends Component {
*/
private final long fBundleId;

/**
* Tracks where/wehn the component was disposed to give better error message
*/
private volatile RuntimeException disposeSource;

/**
* Constructs a new API component from the specified location in the file
* system in the given baseline.
Expand Down Expand Up @@ -183,6 +188,7 @@ public void dispose() {
synchronized (this) {
fManifest = null;
fBundleDescription = null;
disposeSource = new RuntimeException("Component was disposed here"); //$NON-NLS-1$
}
}
}
Expand Down Expand Up @@ -275,28 +281,27 @@ public int hashCode() {
*
* @throws CoreException on failure
*/
protected void init() {
protected void init() throws CoreException {
if (isDisposed() || fBundleDescription != null) {
return;
}
synchronized (this) {
try {
Map<String, String> manifest = getManifest();
if (manifest == null) {
ApiPlugin.log(Status.error("Unable to find a manifest for the component from: " + fLocation, //$NON-NLS-1$
throw new CoreException(
Status.error("Unable to find a manifest for the component from: " + fLocation, //$NON-NLS-1$
null));
return;
}
BundleDescription bundleDescription = getBundleDescription(manifest, fLocation, fBundleId);
fSymbolicName = bundleDescription.getSymbolicName();
fVersion = bundleDescription.getVersion();
setName(manifest.get(Constants.BUNDLE_NAME));
fBundleDescription = bundleDescription;
} catch (BundleException e) {
ApiPlugin.log(Status.error("Unable to create API component from specified location: " + fLocation, //$NON-NLS-1$
throw new CoreException(
Status.error("Unable to create API component from specified location: " + fLocation, //$NON-NLS-1$
e));
} catch (CoreException ce) {
ApiPlugin.log(ce);
}
// compact manifest after initialization - only keep used headers
doManifestCompaction();
Expand Down Expand Up @@ -906,7 +911,11 @@ public String[] getExecutionEnvironments() throws CoreException {

@Override
public final String getSymbolicName() {
init();
try {
init();
} catch (CoreException e) {
ApiPlugin.log(e);
}
return fSymbolicName;
}

Expand All @@ -923,14 +932,26 @@ public IRequiredComponentDescription[] getRequiredComponents() throws CoreExcept

@Override
public String getVersion() {
init();
try {
init();
} catch (CoreException e) {
ApiPlugin.log(e);
}
Version version = fVersion;
if (version== null) {
return null;
}
// without the qualifier
return fVersion.getMajor() + "." + fVersion.getMinor() + "." + fVersion.getMicro(); //$NON-NLS-1$//$NON-NLS-2$
return version.getMajor() + "." + version.getMinor() + "." + version.getMicro(); //$NON-NLS-1$//$NON-NLS-2$
}

@Override
public String getName() {
init();
try {
init();
} catch (CoreException e) {
ApiPlugin.log(e);
}
return super.getName();
}

Expand All @@ -951,13 +972,14 @@ public BundleDescription getBundleDescription() throws CoreException {

@Override
public String toString() {
if (fBundleDescription != null) {
BundleDescription description = fBundleDescription;
if (description != null) {
try {
StringBuilder buffer = new StringBuilder();
buffer.append(fBundleDescription.toString());
buffer.append(description.toString());
buffer.append(" - "); //$NON-NLS-1$
buffer.append("[fragment: ").append(isFragment()).append("] "); //$NON-NLS-1$ //$NON-NLS-2$
buffer.append("[host: ").append(fBundleDescription.getFragments().length > 0).append("] "); //$NON-NLS-1$ //$NON-NLS-2$
buffer.append("[host: ").append(description.getFragments().length > 0).append("] "); //$NON-NLS-1$ //$NON-NLS-2$
buffer.append("[system bundle: ").append(isSystemComponent()).append("] "); //$NON-NLS-1$ //$NON-NLS-2$
buffer.append("[source bundle: ").append(isSourceComponent()).append("] "); //$NON-NLS-1$ //$NON-NLS-2$
buffer.append("[dev bundle: ").append(fWorkspaceBinary).append("]"); //$NON-NLS-1$ //$NON-NLS-2$
Expand Down Expand Up @@ -1207,7 +1229,7 @@ public ResolverError[] getErrors() throws CoreException {
*/
protected void baselineDisposed(IApiBaseline baseline) throws CoreException {
throw new CoreException(new Status(IStatus.ERROR, ApiPlugin.PLUGIN_ID, ApiPlugin.REPORT_BASELINE_IS_DISPOSED,
NLS.bind(Messages.BundleApiComponent_baseline_disposed, getName(), baseline.getName()), null));
NLS.bind(Messages.BundleApiComponent_baseline_disposed, getName(), baseline.getName()), disposeSource));
}

}

0 comments on commit 17ccaed

Please sign in to comment.