Skip to content

cadc-registry: improve client error handling and retry behaviour #236

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

Merged
pdowler merged 4 commits into from
Jun 22, 2026
Merged

cadc-registry: improve client error handling and retry behaviour #236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9b05092
cadc-registry: improve client error handling and retry behaviour
pdowler Jun 16, 2026
ceb00b0
error handling/timing improvements
pdowler Jun 18, 2026
1b2f579
Merge branch 'main' of https://github.com/opencadc/reg
pdowler Jun 18, 2026
0bcf36d
improve javadoc comments wrt. timeouts and thrown exceptions
pdowler Jun 22, 2026
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
4 changes: 2 additions & 2 deletions cadc-registry/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ sourceCompatibility = 11

group = 'org.opencadc'

version = '1.9.1'
version = '1.9.2'

description = 'OpenCADC Registry client library'
def git_url = 'https://github.com/opencadc/reg'

dependencies {
implementation 'org.opencadc:cadc-util:[1.12,)'
implementation 'org.opencadc:cadc-util:[1.12.17,)'

testImplementation 'junit:junit:[4.12,)'
testImplementation 'org.opencadc:cadc-quality:[1.0,)'
Expand Down
21 changes: 20 additions & 1 deletion cadc-registry/src/main/java/ca/nrc/cadc/reg/Capability.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
******************* CANADIAN ASTRONOMY DATA CENTRE *******************
************** CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES **************
*
* (c) 2019. (c) 2019.
* (c) 2026. (c) 2026.
* Government of Canada Gouvernement du Canada
* National Research Council Conseil national de recherches
* Ottawa, Canada, K1A 0R6 Ottawa, Canada, K1A 0R6
Expand Down Expand Up @@ -112,13 +112,29 @@ public List<Interface> getInterfaces() {
return interfaces;
}

/**
* Find the interface with the specified type. If the arg is null, find and return
* the first interface.
* @param interfaceType Standards.INTERFACE_ type constant or null for first interface
* @return matching interface or null if not found
*/
public Interface findInterfaceByType(URI interfaceType) {
Comment thread
pdowler marked this conversation as resolved.
Resolved
for (Interface i : interfaces) {
if (interfaceType == null || interfaceType.equals(i.getType())) {
return i;
}
}
return null;
}

/**
* Find a ParamHTTP interface that uses the specified securityMethod.
* This method returns the first matching interface.
*
* @param securityMethod securityMethod to match
* @return the first matching interface or null
*/
@Deprecated
public Interface findInterface(final URI securityMethod) {
return findInterface(securityMethod, Standards.INTERFACE_PARAM_HTTP);
}
Expand All @@ -131,6 +147,7 @@ public Interface findInterface(final URI securityMethod) {
* @param interfaceType interface type to match
* @return the first matching interface or null
*/
@Deprecated
public Interface findInterface(final URI securityMethod, final URI interfaceType) {
boolean anon = securityMethod == null || Standards.SECURITY_METHOD_ANON.equals(securityMethod);
for (Interface intf : this.interfaces) {
Expand All @@ -155,6 +172,7 @@ public Interface findInterface(final URI securityMethod, final URI interfaceType
* @param authMethod AuthMethod to match
* @return the first matching interface or null
*/
@Deprecated
public Interface findInterface(final AuthMethod authMethod) {
return findInterface(authMethod, Standards.INTERFACE_PARAM_HTTP);
}
Expand All @@ -167,6 +185,7 @@ public Interface findInterface(final AuthMethod authMethod) {
* @param interfaceType interface type to match
* @return the first matching interface or null
*/
@Deprecated
public Interface findInterface(final AuthMethod authMethod, final URI interfaceType) {
URI secMethod = Standards.getSecurityMethod(authMethod);
// TODO: throw if non-null authMethod -> null securityMethod?
Expand Down
56 changes: 42 additions & 14 deletions cadc-registry/src/main/java/ca/nrc/cadc/reg/client/CachingFile.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
******************* CANADIAN ASTRONOMY DATA CENTRE *******************
************** CENTRE CANADIEN DE DONNÉES ASTRONOMIQUES **************
*
* (c) 2023. (c) 2023.
* (c) 2026. (c) 2026.
* Government of Canada Gouvernement du Canada
* National Research Council Conseil national de recherches
* Ottawa, Canada, K1A 0R6 Ottawa, Canada, K1A 0R6
Expand Down Expand Up @@ -71,6 +71,7 @@

import ca.nrc.cadc.net.HttpGet;
import ca.nrc.cadc.net.HttpTransfer;
import ca.nrc.cadc.net.RemoteServiceException;
import ca.nrc.cadc.net.TransientException;
import ca.nrc.cadc.profiler.Profiler;
import java.io.ByteArrayOutputStream;
Expand All @@ -80,6 +81,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.SocketException;
import java.net.URL;
import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.Files;
Expand Down Expand Up @@ -111,13 +113,17 @@ public class CachingFile {

// 10 minutes
private static final int DEFAULT_EXPRIY_SECONDS = 10 * 60;
private int connectionTimeout = 3000; // millis
private int connectionTimeout = 12000; // millis
private int readTimeout = 60000; // millis
private int maxRetries = 3; // for TransientException

private File localCache;
private URL remoteSource;
private long expirySeconds;
private File cacheDir;

// response time of remote service
public Long responseTime = null;

/**
* Construct a caching file with the local file and
Expand Down Expand Up @@ -162,7 +168,7 @@ public CachingFile(File localCache, URL remoteSource, long expirySeconds) {
}

/**
* HTTP connection timeout in milliseconds (default: 30000).
* HTTP connection timeout in milliseconds (default: 12000).
*
* @param connectionTimeout in milliseconds
*/
Expand All @@ -178,6 +184,15 @@ public void setConnectionTimeout(int connectionTimeout) {
public void setReadTimeout(int readTimeout) {
this.readTimeout = readTimeout;
}

/**
* Set the max number of retries for 503 errors.
*
* @param maxRetries maximum number of retries
*/
public void setMaxRetries(int maxRetries) {
this.maxRetries = maxRetries;
}

private File checkCacheDirectory(File cacheFile) {
Profiler profiler = new Profiler(CachingFile.class);
Expand All @@ -187,15 +202,11 @@ private File checkCacheDirectory(File cacheFile) {
log.debug("cache file parent dir: " + dir);

if (dir.exists() && !dir.isDirectory()) {
java.nio.file.Files.delete(dir.toPath());
Files.delete(dir.toPath());
}

if (!dir.exists()) {
// The NIO version seems to create the path properly,
// as opposed to the dir.mkdirs()
// jenkinsd 2017.03.17
//
java.nio.file.Files.createDirectories(dir.toPath());
Files.createDirectories(dir.toPath());
log.debug("Created directory " + dir);
}
return dir;
Expand All @@ -206,7 +217,16 @@ private File checkCacheDirectory(File cacheFile) {
}
}

public String getContent() throws IOException {
/**
* Get the content. This method normally returns the content from the local cache
* and occasionally refreshes the cache from the remote URL (~10 min).
* @return the target content from the remote URL
* @throws IOException if local caching fails
* @throws RemoteServiceException if the call to the remote URL fails
* @throws TransientException if the call fails with a transient exception and retries have already been attempted
* @see setMaxRetries
*/
public String getContent() throws IOException, RemoteServiceException, TransientException {

boolean cacheExists = localCache.exists() && localCache.canRead();
if (cacheExists && !hasExpired()) {
Expand All @@ -221,6 +241,7 @@ public String getContent() throws IOException {
}

// read and cache the remote source
long t1 = System.currentTimeMillis();
try {
// create a temp file in the directory
File tmpFile = File.createTempFile(UUID.randomUUID().toString(), null, cacheDir);
Expand All @@ -229,7 +250,7 @@ public String getContent() throws IOException {
FileOutputStream fos = new FileOutputStream(tmpFile);
try {
loadRemoteContent(fos);
} catch (IOException | TransientException e) {
} catch (IOException e) {
log.warn("Failed to cache " + remoteSource + " to " + cacheDir.getAbsolutePath() + " cause: " + e.getMessage());

log.debug("Deleting tmp cache file because download failed.");
Expand All @@ -242,13 +263,14 @@ public String getContent() throws IOException {

throw e;
} finally {
this.responseTime = System.currentTimeMillis() - t1;
try {
fos.close();
} catch (IOException e) {
log.warn("Failed to close output stream", e);
}
}

// move the file to the real location
Path source = Paths.get(tmpFile.getAbsolutePath());
Path dest = Paths.get(localCache.getAbsolutePath());
Expand Down Expand Up @@ -316,20 +338,27 @@ private String getRemoteContent() throws IOException {
return out.toString("UTF-8");
}

private void loadRemoteContent(OutputStream dest) throws IOException, TransientException {
private void loadRemoteContent(OutputStream dest) throws IOException, RemoteServiceException, TransientException {
Profiler profiler = new Profiler(CachingFile.class);
try {
final long t1 = System.currentTimeMillis();
log.debug("loadRemoteContent: " + remoteSource + " " + connectionTimeout + "/" + readTimeout);
HttpGet download = new HttpGet(remoteSource, dest);
download.setConnectionTimeout(connectionTimeout);
download.setReadTimeout(readTimeout);
download.setMaxRetries(maxRetries);
download.run();
long dt = System.currentTimeMillis() - t1;
log.debug("loadRemoteContent: dt=" + dt);

if (download.getThrowable() != null) {
log.warn("Could not get source from " + remoteSource + ": " + download.getThrowable());
if (download.getThrowable() instanceof TransientException) {
throw (TransientException) download.getThrowable();
}
if (download.getThrowable() instanceof RemoteServiceException) {
throw (RemoteServiceException) download.getThrowable();
}
throw new IOException(download.getThrowable());
}
} finally {
Expand All @@ -343,5 +372,4 @@ private boolean hasExpired() {
long now = System.currentTimeMillis();
return ((now - lastModified) > expiryMillis);
}

}
Loading
Loading