Skip to content

Fix silent error handling in FTP listener #1549

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
niveathika merged 5 commits into from
Feb 15, 2026
Merged

Fix silent error handling in FTP listener #1549

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7098fc4
Add upfront regex pattern validation in FormatMethodsHolder
niveathika Feb 14, 2026
f37b948
Replace log.error with BError.printStackTrace for visible errors
niveathika Feb 14, 2026
2997953
Remove redundant code in FtpListener
niveathika Feb 14, 2026
c17d593
Remove unnecessary log statements
niveathika Feb 14, 2026
866fed5
Update changelog
niveathika Feb 14, 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
2 changes: 0 additions & 2 deletions native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClientHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ static boolean executeGetAction(RemoteFileSystemBaseMessage remoteFileSystemBase
balFuture.complete(streamEntry);
}
} catch (IOException e) {
log.error("{}", FtpConstants.ERR_READING_STREAM, e);
balFuture.complete(FtpUtil.createError(FtpConstants.ERR_READING_STREAM, e, FTP_ERROR));
}
return true;
Expand Down Expand Up @@ -130,7 +129,6 @@ private static Object createStreamWithContent(InputStream content, Type streamVa
return ContentCsvStreamIteratorUtils.createRecordStream(content, streamValueType,
laxDataBinding, null);
} catch (Exception e) {
log.error("Failed to create stream with content", e);
return FtpUtil.createError(FtpConstants.ERR_CREATE_STREAM, e, FTP_ERROR);
}
}
Expand Down
1 change: 0 additions & 1 deletion native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClientListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ public boolean onMessage(RemoteFileSystemBaseMessage remoteFileSystemBaseMessage

@Override
public void onError(Throwable throwable) {
log.error(throwable.getMessage(), throwable);
String errorType = FtpUtil.getErrorTypeForException(throwable);
balFuture.complete(FtpUtil.createError(throwable.getMessage(), throwable.getCause(), errorType));
}
Expand Down
11 changes: 5 additions & 6 deletions native/src/main/java/io/ballerina/stdlib/ftp/server/FormatMethodsHolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ private void initializeMethodMappings() throws FtpInvalidConfigException {
Object patternObj = annotation.get(StringUtils.fromString(ANNOTATION_PATTERN_FIELD));
if (patternObj != null) {
String pattern = patternObj.toString();
FtpUtil.validateRegexPattern(pattern,
"@ftp:FunctionConfig.fileNamePattern for method '" + methodName + "'");
annotationPatternToMethod.put(pattern, method);
log.debug("Registered annotation pattern '{}' for method '{}'", pattern, methodName);
}
Expand Down Expand Up @@ -207,12 +209,9 @@ public Optional<MethodType> getMethod(FileInfo fileInfo) {
private Optional<MethodType> findMethodByAnnotationPattern(String fileName) {
for (Map.Entry<String, MethodType> entry : annotationPatternToMethod.entrySet()) {
String pattern = entry.getKey();
try {
if (Pattern.matches(pattern, fileName)) {
return Optional.of(entry.getValue());
}
} catch (Exception e) {
log.warn("Invalid regex pattern '{}' in FileConfig annotation: {}", pattern, e.getMessage());
// Patterns are validated during initialization, so no try-catch needed
if (Pattern.matches(pattern, fileName)) {
return Optional.of(entry.getValue());
}
}
return Optional.empty();
Expand Down
27 changes: 16 additions & 11 deletions native/src/main/java/io/ballerina/stdlib/ftp/server/FtpContentCallbackHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ public void processContentCallbacks(Environment env, BObject service, RemoteFile

// Check if content conversion returned an error (ContentBindingError)
if (convertedContent instanceof BError bError) {
log.error("Content binding failed for file: {}", fileInfo.getPath());
routeToOnError(service, holder, bError, callerObject, fileInfo, listenerPath);
continue;
}
Expand All @@ -139,7 +138,8 @@ public void processContentCallbacks(Environment env, BObject service, RemoteFile
fileInfo, callerObject, listenerPath, afterProcess, afterError);

} catch (Exception exception) {
log.error("Failed to process file: " + fileInfo.getPath(), exception);
FtpUtil.createError("Failed to process file: " + fileInfo.getPath() + " - " + exception.getMessage(),
exception, FtpConstants.FTP_ERROR).printStackTrace();
// Continue processing other files even if one fails
}
}
Expand Down Expand Up @@ -276,11 +276,13 @@ private void routeToOnError(BObject service, FormatMethodsHolder holder, BError
FileInfo fileInfo, String listenerPath) {
if (!holder.hasOnErrorMethod()) {
// No onError handler, error is already logged
error.printStackTrace();
return;
}

Optional<MethodType> onErrorMethodOpt = holder.getOnErrorMethod();
if (onErrorMethodOpt.isEmpty()) {
error.printStackTrace();
return;
}

Expand Down Expand Up @@ -332,7 +334,8 @@ private void invokeOnErrorMethodAsync(BObject service, String methodName, Object
} catch (BError error) {
error.printStackTrace();
} catch (Exception exception) {
log.error("Error invoking onError method: " + methodName, exception);
FtpUtil.createError("Error invoking onError method: " + methodName + " - " + exception.getMessage(),
exception, FtpConstants.FTP_ERROR).printStackTrace();
}

if (isSuccess) {
Expand Down Expand Up @@ -372,7 +375,8 @@ private void invokeContentMethodAsync(BObject service, String methodName, Object
afterError.ifPresent(action -> executePostProcessAction(action, fileInfo, callerObject,
listenerPath, "afterError"));
} catch (Exception exception) {
log.error("Error invoking content method: " + methodName, exception);
FtpUtil.createError("Error invoking content method: " + methodName + " - " + exception.getMessage(),
exception, FtpConstants.FTP_ERROR).printStackTrace();
// Method threw an exception - execute afterError action
afterError.ifPresent(action -> executePostProcessAction(action, fileInfo, callerObject,
listenerPath, "afterError"));
Expand Down Expand Up @@ -405,7 +409,8 @@ private void executePostProcessAction(PostProcessAction action, FileInfo fileInf
executeMoveAction(callerObject, filePath, listenerPath, action, actionContext);
}
} catch (Exception e) {
log.error("Failed to execute {} action on file: {}", actionContext, filePath, e);
FtpUtil.createError("Failed to execute " + actionContext + " action on file: " + filePath +
" - " + e.getMessage(), e, FtpConstants.FTP_ERROR).printStackTrace();
}
}

Expand All @@ -417,13 +422,13 @@ private void executeDeleteAction(BObject callerObject, String filePath, String a
StringUtils.fromString(filePath));

if (result instanceof BError) {
log.error("Failed to delete file during {}: {} - {}", actionContext, filePath,
((BError) result).getErrorMessage());
((BError) result).printStackTrace();
} else {
log.debug("Successfully deleted file during {}: {}", actionContext, filePath);
}
} catch (Exception e) {
log.error("Exception during delete action ({}): {}", actionContext, filePath, e);
FtpUtil.createError("Exception during delete action (" + actionContext + "): " + filePath +
" - " + e.getMessage(), e, FtpConstants.FTP_ERROR).printStackTrace();
}
}

Expand All @@ -438,13 +443,13 @@ private void executeMoveAction(BObject callerObject, String filePath, String lis
StringUtils.fromString(filePath), StringUtils.fromString(destinationPath));

if (result instanceof BError) {
log.error("Failed to move file during {}: {} -> {} - {}", actionContext, filePath,
destinationPath, ((BError) result).getErrorMessage());
((BError) result).printStackTrace();
} else {
log.debug("Successfully moved file during {}: {} -> {}", actionContext, filePath, destinationPath);
}
} catch (Exception e) {
log.error("Exception during move action ({}): {}", actionContext, filePath, e);
FtpUtil.createError("Exception during move action (" + actionContext + "): " + filePath +
" - " + e.getMessage(), e, FtpConstants.FTP_ERROR).printStackTrace();
}
}

Expand Down
23 changes: 5 additions & 18 deletions native/src/main/java/io/ballerina/stdlib/ftp/server/FtpListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,8 @@ private void dispatchFileEventToService(Environment env, ServiceContext context,
formatMethodHolder = new FormatMethodsHolder(service);
} catch (FtpInvalidConfigException e) {
// This should not happen as validation occurs during attach
log.error("Invalid post-process action configuration: {}", e.getMessage());
FtpUtil.createError("Invalid post-process action configuration: " + e.getMessage(),
e, FtpConstants.FTP_ERROR).printStackTrace();
return;
}
}
Expand All @@ -170,14 +171,7 @@ private void dispatchFileEventToService(Environment env, ServiceContext context,
} else {
// Strategy 3: Fall back to legacy onFileChange handler
Optional<MethodType> onFileChangeMethodType = getOnFileChangeMethod(service);
if (onFileChangeMethodType.isPresent()) {
log.debug("Service uses deprecated onFileChange handler for file events.");
processMetadataOnlyCallbacks(service, event, onFileChangeMethodType.get(), caller);
} else {
log.error("Service has no valid handler method. Must have one of: " +
"onFile, onFileText, onFileJson, onFileXml, onFileCsv (format-specific), " +
"onFileDeleted, or onFileChange (deprecated)");
}
processMetadataOnlyCallbacks(service, event, onFileChangeMethodType.get(), caller);
}
}

Expand All @@ -199,7 +193,8 @@ private void processContentBasedCallbacks(Environment env, BObject service, Remo
runtime, fileSystemManager, fileSystemOptions, laxDataBinding, csvFailSafe);
contentHandler.processContentCallbacks(env, service, event, holder, caller);
} catch (Exception e) {
log.error("Error in content callback processing for added files", e);
FtpUtil.createError("Error in content callback processing for added files: " + e.getMessage(),
e, FtpConstants.FTP_ERROR).printStackTrace();
}
}
}
Expand Down Expand Up @@ -297,11 +292,7 @@ private Object[] getMethodArguments(Parameter[] params, Map<String, Object> watc
} else if ((params[1].type.isReadOnly() || TypeUtils.getReferredType(params[1].type).getTag() ==
RECORD_TYPE_TAG) && TypeUtils.getReferredType(params[0].type).getTag() == OBJECT_TYPE_TAG) {
return new Object[] {caller, getWatchEvent(params[1], watchEventParamValues)};
} else {
log.error("Invalid parameter types in onFileChange method");
}
} else {
log.error("Invalid parameter count in onFileChange method");
}
return null;
}
Expand All @@ -313,8 +304,6 @@ private Object[] getOnFileDeleteMethodArguments(Parameter[] params, BString dele
} else if (params.length == 2) {
// deletedFile and caller parameters
return new Object[] {deletedFile, caller};
} else {
log.error("Invalid parameter count in onFileDelete method");
}
return null;
}
Expand All @@ -326,8 +315,6 @@ private Object[] getOnFileDeletedMethodArguments(Parameter[] params, BArray dele
} else if (params.length == 2) {
// deletedFiles and caller parameters
return new Object[] {deletedFiles, caller};
} else {
log.error("Invalid parameter count in onFileDeleted method");
}
return null;
}
Expand Down
4 changes: 0 additions & 4 deletions native/src/main/java/io/ballerina/stdlib/ftp/util/FtpContentConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,13 +93,11 @@ public static Object convertBytesToJson(byte[] content, Type targetType, boolean

Object result = io.ballerina.lib.data.jsondata.json.Native.parseBytes(byteArray, options, typedesc);
if (result instanceof BError bError) {
log.error("Failed to parse JSON content: {}", bError.getMessage());
return FtpUtil.createContentBindingError(bError.getErrorMessage().getValue(), bError, filePath,
content);
}
return result;
} catch (Exception e) {
log.error("Error converting bytes to JSON", e);
return FtpUtil.createContentBindingError("Failed to parse JSON content: " + e.getMessage(), e,
filePath, content);
}
Expand Down Expand Up @@ -157,14 +155,12 @@ public static Object convertBytesToCsv(Environment env, byte[] content, Type tar
Object result = parseBytes(env, byteArray, options, typedesc);

if (result instanceof BError bError) {
log.error("Failed to parse CSV content: {}", bError.getMessage());
return FtpUtil.createContentBindingError("Failed to parse CSV content: " + bError.getErrorMessage(),
bError, filePath, content);
}

return result;
} catch (Exception e) {
log.error("Error converting bytes to CSV", e);
return FtpUtil.createContentBindingError("Failed to parse CSV content: " + e.getMessage(), e,
filePath, content);
}
Expand Down
1 change: 0 additions & 1 deletion native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,6 @@ public static Throwable findRootCause(Throwable throwable) {
*/
public static int extractPortValue(long longValue) {
if (longValue <= 0 || longValue > MAX_PORT) {
log.error("Invalid port number given in configuration");
return -1;
}
try {
Expand Down
Loading