Skip to content

Commit 883221b

Browse files
authored
Merge pull request #1549 from ballerina-platform/fix-silent-error-handling
Fix silent error handling in FTP listener
2 parents 0f4d1b1 + 866fed5 commit 883221b

File tree

8 files changed

+27
-44
lines changed

8 files changed

+27
-44
lines changed

changelog.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
1717
- [Incorporate the csv fail safe support in the FTP listener](https://github.com/ballerina-platform/ballerina-library/issues/8502)
1818

1919
### Fixed
20-
20+
- [Fix Listener silently fails when errors occur](https://github.com/ballerina-platform/ballerina-library/issues/8655)
2121

2222
## [2.16.0] - 2025-12-19
2323

native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClientHelper.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ static boolean executeGetAction(RemoteFileSystemBaseMessage remoteFileSystemBase
9999
balFuture.complete(streamEntry);
100100
}
101101
} catch (IOException e) {
102-
log.error("{}", FtpConstants.ERR_READING_STREAM, e);
103102
balFuture.complete(FtpUtil.createError(FtpConstants.ERR_READING_STREAM, e, FTP_ERROR));
104103
}
105104
return true;
@@ -130,7 +129,6 @@ private static Object createStreamWithContent(InputStream content, Type streamVa
130129
return ContentCsvStreamIteratorUtils.createRecordStream(content, streamValueType,
131130
laxDataBinding, null);
132131
} catch (Exception e) {
133-
log.error("Failed to create stream with content", e);
134132
return FtpUtil.createError(FtpConstants.ERR_CREATE_STREAM, e, FTP_ERROR);
135133
}
136134
}

native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClientListener.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public boolean onMessage(RemoteFileSystemBaseMessage remoteFileSystemBaseMessage
5454

5555
@Override
5656
public void onError(Throwable throwable) {
57-
log.error(throwable.getMessage(), throwable);
5857
String errorType = FtpUtil.getErrorTypeForException(throwable);
5958
balFuture.complete(FtpUtil.createError(throwable.getMessage(), throwable.getCause(), errorType));
6059
}

native/src/main/java/io/ballerina/stdlib/ftp/server/FormatMethodsHolder.java

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ private void initializeMethodMappings() throws FtpInvalidConfigException {
8686
Object patternObj = annotation.get(StringUtils.fromString(ANNOTATION_PATTERN_FIELD));
8787
if (patternObj != null) {
8888
String pattern = patternObj.toString();
89+
FtpUtil.validateRegexPattern(pattern,
90+
"@ftp:FunctionConfig.fileNamePattern for method '" + methodName + "'");
8991
annotationPatternToMethod.put(pattern, method);
9092
log.debug("Registered annotation pattern '{}' for method '{}'", pattern, methodName);
9193
}
@@ -207,12 +209,9 @@ public Optional<MethodType> getMethod(FileInfo fileInfo) {
207209
private Optional<MethodType> findMethodByAnnotationPattern(String fileName) {
208210
for (Map.Entry<String, MethodType> entry : annotationPatternToMethod.entrySet()) {
209211
String pattern = entry.getKey();
210-
try {
211-
if (Pattern.matches(pattern, fileName)) {
212-
return Optional.of(entry.getValue());
213-
}
214-
} catch (Exception e) {
215-
log.warn("Invalid regex pattern '{}' in FileConfig annotation: {}", pattern, e.getMessage());
212+
// Patterns are validated during initialization, so no try-catch needed
213+
if (Pattern.matches(pattern, fileName)) {
214+
return Optional.of(entry.getValue());
216215
}
217216
}
218217
return Optional.empty();

native/src/main/java/io/ballerina/stdlib/ftp/server/FtpContentCallbackHandler.java

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,6 @@ public void processContentCallbacks(Environment env, BObject service, RemoteFile
121121

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

141140
} catch (Exception exception) {
142-
log.error("Failed to process file: " + fileInfo.getPath(), exception);
141+
FtpUtil.createError("Failed to process file: " + fileInfo.getPath() + " - " + exception.getMessage(),
142+
exception, FtpConstants.FTP_ERROR).printStackTrace();
143143
// Continue processing other files even if one fails
144144
}
145145
}
@@ -276,11 +276,13 @@ private void routeToOnError(BObject service, FormatMethodsHolder holder, BError
276276
FileInfo fileInfo, String listenerPath) {
277277
if (!holder.hasOnErrorMethod()) {
278278
// No onError handler, error is already logged
279+
error.printStackTrace();
279280
return;
280281
}
281282

282283
Optional<MethodType> onErrorMethodOpt = holder.getOnErrorMethod();
283284
if (onErrorMethodOpt.isEmpty()) {
285+
error.printStackTrace();
284286
return;
285287
}
286288

@@ -332,7 +334,8 @@ private void invokeOnErrorMethodAsync(BObject service, String methodName, Object
332334
} catch (BError error) {
333335
error.printStackTrace();
334336
} catch (Exception exception) {
335-
log.error("Error invoking onError method: " + methodName, exception);
337+
FtpUtil.createError("Error invoking onError method: " + methodName + " - " + exception.getMessage(),
338+
exception, FtpConstants.FTP_ERROR).printStackTrace();
336339
}
337340

338341
if (isSuccess) {
@@ -372,7 +375,8 @@ private void invokeContentMethodAsync(BObject service, String methodName, Object
372375
afterError.ifPresent(action -> executePostProcessAction(action, fileInfo, callerObject,
373376
listenerPath, "afterError"));
374377
} catch (Exception exception) {
375-
log.error("Error invoking content method: " + methodName, exception);
378+
FtpUtil.createError("Error invoking content method: " + methodName + " - " + exception.getMessage(),
379+
exception, FtpConstants.FTP_ERROR).printStackTrace();
376380
// Method threw an exception - execute afterError action
377381
afterError.ifPresent(action -> executePostProcessAction(action, fileInfo, callerObject,
378382
listenerPath, "afterError"));
@@ -405,7 +409,8 @@ private void executePostProcessAction(PostProcessAction action, FileInfo fileInf
405409
executeMoveAction(callerObject, filePath, listenerPath, action, actionContext);
406410
}
407411
} catch (Exception e) {
408-
log.error("Failed to execute {} action on file: {}", actionContext, filePath, e);
412+
FtpUtil.createError("Failed to execute " + actionContext + " action on file: " + filePath +
413+
" - " + e.getMessage(), e, FtpConstants.FTP_ERROR).printStackTrace();
409414
}
410415
}
411416

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

419424
if (result instanceof BError) {
420-
log.error("Failed to delete file during {}: {} - {}", actionContext, filePath,
421-
((BError) result).getErrorMessage());
425+
((BError) result).printStackTrace();
422426
} else {
423427
log.debug("Successfully deleted file during {}: {}", actionContext, filePath);
424428
}
425429
} catch (Exception e) {
426-
log.error("Exception during delete action ({}): {}", actionContext, filePath, e);
430+
FtpUtil.createError("Exception during delete action (" + actionContext + "): " + filePath +
431+
" - " + e.getMessage(), e, FtpConstants.FTP_ERROR).printStackTrace();
427432
}
428433
}
429434

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

440445
if (result instanceof BError) {
441-
log.error("Failed to move file during {}: {} -> {} - {}", actionContext, filePath,
442-
destinationPath, ((BError) result).getErrorMessage());
446+
((BError) result).printStackTrace();
443447
} else {
444448
log.debug("Successfully moved file during {}: {} -> {}", actionContext, filePath, destinationPath);
445449
}
446450
} catch (Exception e) {
447-
log.error("Exception during move action ({}): {}", actionContext, filePath, e);
451+
FtpUtil.createError("Exception during move action (" + actionContext + "): " + filePath +
452+
" - " + e.getMessage(), e, FtpConstants.FTP_ERROR).printStackTrace();
448453
}
449454
}
450455

native/src/main/java/io/ballerina/stdlib/ftp/server/FtpListener.java

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,8 @@ private void dispatchFileEventToService(Environment env, ServiceContext context,
154154
formatMethodHolder = new FormatMethodsHolder(service);
155155
} catch (FtpInvalidConfigException e) {
156156
// This should not happen as validation occurs during attach
157-
log.error("Invalid post-process action configuration: {}", e.getMessage());
157+
FtpUtil.createError("Invalid post-process action configuration: " + e.getMessage(),
158+
e, FtpConstants.FTP_ERROR).printStackTrace();
158159
return;
159160
}
160161
}
@@ -170,14 +171,7 @@ private void dispatchFileEventToService(Environment env, ServiceContext context,
170171
} else {
171172
// Strategy 3: Fall back to legacy onFileChange handler
172173
Optional<MethodType> onFileChangeMethodType = getOnFileChangeMethod(service);
173-
if (onFileChangeMethodType.isPresent()) {
174-
log.debug("Service uses deprecated onFileChange handler for file events.");
175-
processMetadataOnlyCallbacks(service, event, onFileChangeMethodType.get(), caller);
176-
} else {
177-
log.error("Service has no valid handler method. Must have one of: " +
178-
"onFile, onFileText, onFileJson, onFileXml, onFileCsv (format-specific), " +
179-
"onFileDeleted, or onFileChange (deprecated)");
180-
}
174+
processMetadataOnlyCallbacks(service, event, onFileChangeMethodType.get(), caller);
181175
}
182176
}
183177

@@ -199,7 +193,8 @@ private void processContentBasedCallbacks(Environment env, BObject service, Remo
199193
runtime, fileSystemManager, fileSystemOptions, laxDataBinding, csvFailSafe);
200194
contentHandler.processContentCallbacks(env, service, event, holder, caller);
201195
} catch (Exception e) {
202-
log.error("Error in content callback processing for added files", e);
196+
FtpUtil.createError("Error in content callback processing for added files: " + e.getMessage(),
197+
e, FtpConstants.FTP_ERROR).printStackTrace();
203198
}
204199
}
205200
}
@@ -297,11 +292,7 @@ private Object[] getMethodArguments(Parameter[] params, Map<String, Object> watc
297292
} else if ((params[1].type.isReadOnly() || TypeUtils.getReferredType(params[1].type).getTag() ==
298293
RECORD_TYPE_TAG) && TypeUtils.getReferredType(params[0].type).getTag() == OBJECT_TYPE_TAG) {
299294
return new Object[] {caller, getWatchEvent(params[1], watchEventParamValues)};
300-
} else {
301-
log.error("Invalid parameter types in onFileChange method");
302295
}
303-
} else {
304-
log.error("Invalid parameter count in onFileChange method");
305296
}
306297
return null;
307298
}
@@ -313,8 +304,6 @@ private Object[] getOnFileDeleteMethodArguments(Parameter[] params, BString dele
313304
} else if (params.length == 2) {
314305
// deletedFile and caller parameters
315306
return new Object[] {deletedFile, caller};
316-
} else {
317-
log.error("Invalid parameter count in onFileDelete method");
318307
}
319308
return null;
320309
}
@@ -326,8 +315,6 @@ private Object[] getOnFileDeletedMethodArguments(Parameter[] params, BArray dele
326315
} else if (params.length == 2) {
327316
// deletedFiles and caller parameters
328317
return new Object[] {deletedFiles, caller};
329-
} else {
330-
log.error("Invalid parameter count in onFileDeleted method");
331318
}
332319
return null;
333320
}

native/src/main/java/io/ballerina/stdlib/ftp/util/FtpContentConverter.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,11 @@ public static Object convertBytesToJson(byte[] content, Type targetType, boolean
9393

9494
Object result = io.ballerina.lib.data.jsondata.json.Native.parseBytes(byteArray, options, typedesc);
9595
if (result instanceof BError bError) {
96-
log.error("Failed to parse JSON content: {}", bError.getMessage());
9796
return FtpUtil.createContentBindingError(bError.getErrorMessage().getValue(), bError, filePath,
9897
content);
9998
}
10099
return result;
101100
} catch (Exception e) {
102-
log.error("Error converting bytes to JSON", e);
103101
return FtpUtil.createContentBindingError("Failed to parse JSON content: " + e.getMessage(), e,
104102
filePath, content);
105103
}
@@ -157,14 +155,12 @@ public static Object convertBytesToCsv(Environment env, byte[] content, Type tar
157155
Object result = parseBytes(env, byteArray, options, typedesc);
158156

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

165162
return result;
166163
} catch (Exception e) {
167-
log.error("Error converting bytes to CSV", e);
168164
return FtpUtil.createContentBindingError("Failed to parse CSV content: " + e.getMessage(), e,
169165
filePath, content);
170166
}

native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,6 @@ public static Throwable findRootCause(Throwable throwable) {
458458
*/
459459
public static int extractPortValue(long longValue) {
460460
if (longValue <= 0 || longValue > MAX_PORT) {
461-
log.error("Invalid port number given in configuration");
462461
return -1;
463462
}
464463
try {

0 commit comments

Comments
 (0)