From 4cc2f822a65e203fc6331a35274b8e870c71d0e0 Mon Sep 17 00:00:00 2001 From: Niveathika Date: Thu, 5 Feb 2026 10:41:42 +0530 Subject: [PATCH 1/6] Add compiler plugin validation for onError --- .../ftp/plugin/FtpServiceValidationTest.java | 63 ++++++++++ .../invalid_on_error_service_1/Ballerina.toml | 4 + .../invalid_on_error_service_1/service.bal | 37 ++++++ .../invalid_on_error_service_3/Ballerina.toml | 4 + .../invalid_on_error_service_3/service.bal | 37 ++++++ .../invalid_on_error_service_4/Ballerina.toml | 4 + .../invalid_on_error_service_4/service.bal | 37 ++++++ .../valid_on_error_service_1/Ballerina.toml | 4 + .../valid_on_error_service_1/service.bal | 37 ++++++ .../valid_on_error_service_2/Ballerina.toml | 4 + .../valid_on_error_service_2/service.bal | 37 ++++++ .../valid_on_error_service_3/Ballerina.toml | 4 + .../valid_on_error_service_3/service.bal | 37 ++++++ .../ftp/plugin/FtpOnErrorValidator.java | 118 ++++++++++++++++++ .../ftp/plugin/FtpServiceValidator.java | 10 ++ .../stdlib/ftp/plugin/PluginConstants.java | 13 +- spotbugs-exclude.xml | 4 + 17 files changed, 453 insertions(+), 1 deletion(-) create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/service.bal create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/service.bal create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/service.bal create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/service.bal create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/service.bal create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/service.bal create mode 100644 compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java index 8e70e9026..2bd342729 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java @@ -50,8 +50,11 @@ import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.INVALID_ON_FILE_DELETE_PARAMETER; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.TOO_MANY_PARAMETERS_ON_FILE_DELETE; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.BOTH_ON_FILE_DELETE_METHODS_NOT_ALLOWED; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.INVALID_ON_ERROR_SECOND_PARAMETER; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.ON_ERROR_MUST_BE_REMOTE; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.RESOURCE_FUNCTION_NOT_ALLOWED; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.TOO_MANY_PARAMETERS; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.TOO_MANY_PARAMETERS_ON_ERROR; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.TOO_MANY_PARAMETERS_ON_FILE_DELETED; /** @@ -730,4 +733,64 @@ public void testInvalidContentService27() { "Mandatory parameter missing for onFileDelete. Expected string."); } + // ==================== onError Handler Tests ==================== + + @Test(description = "Validation with valid onError handler (Error only)") + public void testValidOnErrorService1() { + Package currentPackage = loadPackage("valid_on_error_service_1"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 0); + } + + @Test(description = "Validation with valid onError handler (Error and Caller)") + public void testValidOnErrorService2() { + Package currentPackage = loadPackage("valid_on_error_service_2"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 0); + } + + @Test(description = "Validation with valid onError handler (error)") + public void testValidOnErrorService3() { + Package currentPackage = loadPackage("valid_on_error_service_3"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 0); + } + + @Test(description = "Validation when onError method is not remote") + public void testInvalidOnErrorService1() { + Package currentPackage = loadPackage("invalid_on_error_service_1"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 1); + Diagnostic diagnostic = (Diagnostic) diagnosticResult.errors().toArray()[0]; + assertDiagnostic(diagnostic, ON_ERROR_MUST_BE_REMOTE, + "onError method must be remote."); + } + + @Test(description = "Validation when onError has invalid second parameter (not Caller)") + public void testInvalidOnErrorService3() { + Package currentPackage = loadPackage("invalid_on_error_service_3"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 1); + Diagnostic diagnostic = (Diagnostic) diagnosticResult.errors().toArray()[0]; + assertDiagnostic(diagnostic, INVALID_ON_ERROR_SECOND_PARAMETER, + "Invalid second parameter for onError. Second parameter must be ftp:Caller."); + } + + @Test(description = "Validation when onError has too many parameters") + public void testInvalidOnErrorService4() { + Package currentPackage = loadPackage("invalid_on_error_service_4"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 1); + Diagnostic diagnostic = (Diagnostic) diagnosticResult.errors().toArray()[0]; + assertDiagnostic(diagnostic, TOO_MANY_PARAMETERS_ON_ERROR, + "Too many parameters for onError. Accepts at most 2 parameters: " + + "(error, caller?)."); + } + } diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/Ballerina.toml new file mode 100644 index 000000000..1279d82a4 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "ftp_test" +name = "invalid_on_error_service_1" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/service.bal new file mode 100644 index 000000000..a99e31fcb --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_1/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Invalid: onError is not remote +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo) returns error? { + return; + } + + function onError(ftp:Error err) returns error? { + return; + } +} diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/Ballerina.toml new file mode 100644 index 000000000..8d697a3d5 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "ftp_test" +name = "invalid_on_error_service_3" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/service.bal new file mode 100644 index 000000000..6bdae417f --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_3/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Invalid: second parameter is not Caller +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo) returns error? { + return; + } + + remote function onError(ftp:Error err, ftp:FileInfo fileInfo) returns error? { + return; + } +} diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/Ballerina.toml new file mode 100644 index 000000000..f67a6dd54 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "ftp_test" +name = "invalid_on_error_service_4" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/service.bal new file mode 100644 index 000000000..e7098d9c7 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/invalid_on_error_service_4/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Invalid: too many parameters +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo) returns error? { + return; + } + + remote function onError(ftp:Error err, ftp:Caller caller, ftp:FileInfo fileInfo) returns error? { + return; + } +} diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/Ballerina.toml new file mode 100644 index 000000000..e12b1b27b --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "ftp_test" +name = "valid_on_error_service_1" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/service.bal new file mode 100644 index 000000000..becb5f60a --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_1/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Valid service with onError handler (ContentBindingError only) +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo) returns error? { + return; + } + + remote function onError(ftp:Error err) returns error? { + return; + } +} diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/Ballerina.toml new file mode 100644 index 000000000..d4234d57a --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "ftp_test" +name = "valid_on_error_service_2" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/service.bal new file mode 100644 index 000000000..29eb94e77 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_2/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Valid service with onError handler (ContentBindingError and Caller) +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo, ftp:Caller caller) returns error? { + return; + } + + remote function onError(ftp:Error err, ftp:Caller caller) returns error? { + return; + } +} diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/Ballerina.toml new file mode 100644 index 000000000..2c7bd844d --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "ftp_test" +name = "invalid_on_error_service_2" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/service.bal new file mode 100644 index 000000000..6b64c6f22 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_3/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Invalid: first parameter is not ContentBindingError +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo) returns error? { + return; + } + + remote function onError(error err) returns error? { + return; + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java new file mode 100644 index 000000000..b008cfa15 --- /dev/null +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java @@ -0,0 +1,118 @@ +/* + * Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. + * + * WSO2 LLC. licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file except + * in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.ballerina.stdlib.ftp.plugin; + +import io.ballerina.compiler.api.SemanticModel; +import io.ballerina.compiler.api.symbols.Symbol; +import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode; +import io.ballerina.compiler.syntax.tree.Node; +import io.ballerina.compiler.syntax.tree.ParameterNode; +import io.ballerina.compiler.syntax.tree.RequiredParameterNode; +import io.ballerina.compiler.syntax.tree.SeparatedNodeList; +import io.ballerina.compiler.syntax.tree.SyntaxKind; +import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; +import io.ballerina.tools.diagnostics.DiagnosticSeverity; + +import java.util.Optional; + +import static io.ballerina.compiler.syntax.tree.SyntaxKind.ERROR_TYPE_DESC; +import static io.ballerina.compiler.syntax.tree.SyntaxKind.QUALIFIED_NAME_REFERENCE; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.ERROR_PARAM; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.INVALID_ON_ERROR_FIRST_PARAMETER; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.INVALID_ON_ERROR_SECOND_PARAMETER; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.ON_ERROR_MUST_BE_REMOTE; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.TOO_MANY_PARAMETERS_ON_ERROR; +import static io.ballerina.stdlib.ftp.plugin.PluginUtils.getDiagnostic; +import static io.ballerina.stdlib.ftp.plugin.PluginUtils.isRemoteFunction; +import static io.ballerina.stdlib.ftp.plugin.PluginUtils.validateModuleId; + +/** + * Validates the onError remote function in FTP service. + */ +public class FtpOnErrorValidator { + + private final SyntaxNodeAnalysisContext context; + private final FunctionDefinitionNode functionDefinitionNode; + + public FtpOnErrorValidator(SyntaxNodeAnalysisContext context, FunctionDefinitionNode functionDefinitionNode) { + this.context = context; + this.functionDefinitionNode = functionDefinitionNode; + } + + public void validate() { + // Check if remote + if (!isRemoteFunction(context, functionDefinitionNode)) { + context.reportDiagnostic(getDiagnostic(ON_ERROR_MUST_BE_REMOTE, + DiagnosticSeverity.ERROR, functionDefinitionNode.location())); + return; + } + + SeparatedNodeList parameters = functionDefinitionNode.functionSignature().parameters(); + int paramCount = parameters.size(); + + if (paramCount == 0) { + context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_FIRST_PARAMETER, + DiagnosticSeverity.ERROR, functionDefinitionNode.location())); + return; + } + + // Validate parameter count (max 2: error, caller?) + if (paramCount > 2) { + context.reportDiagnostic(getDiagnostic(TOO_MANY_PARAMETERS_ON_ERROR, + DiagnosticSeverity.ERROR, functionDefinitionNode.location())); + return; + } + + // Validate first parameter - must be ftp:Error or error + ParameterNode firstParamNode = parameters.get(0); + validateErrorParameter(firstParamNode); + + // Validate 2nd parameter if present - must be Caller + if (paramCount == 2) { + ParameterNode secondParamNode = parameters.get(1); + if (!PluginUtils.validateCallerParameter(secondParamNode, context)) { + context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_SECOND_PARAMETER, + DiagnosticSeverity.ERROR, secondParamNode.location())); + } + } + + // Validate return type - must be error? + PluginUtils.validateReturnTypeErrorOrNil(functionDefinitionNode, context); + } + + private void validateErrorParameter(ParameterNode parameterNode) { + SyntaxKind paramSyntaxKind = ((RequiredParameterNode) parameterNode).typeName().kind(); + SemanticModel semanticModel = context.semanticModel(); + if (paramSyntaxKind.equals(QUALIFIED_NAME_REFERENCE)) { + Node parameterTypeNode = ((RequiredParameterNode) parameterNode).typeName(); + Optional paramSymbol = semanticModel.symbol(parameterTypeNode); + if (paramSymbol.isEmpty() || paramSymbol.get().getName().isEmpty() || + !paramSymbol.get().getName().get().equals(ERROR_PARAM) || + paramSymbol.get().getModule().isEmpty() || + !validateModuleId(paramSymbol.get().getModule().get())) { + context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_FIRST_PARAMETER, + DiagnosticSeverity.ERROR, parameterNode.location())); + } + } else if (!paramSyntaxKind.equals(ERROR_TYPE_DESC)) { + context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_FIRST_PARAMETER, + DiagnosticSeverity.ERROR, parameterNode.location())); + } + } + +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidator.java index ed0f825a4..b3ef15552 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidator.java @@ -40,6 +40,7 @@ import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.NO_VALID_REMOTE_METHOD; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.ON_FILE_CHANGE_DEPRECATED; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.RESOURCE_FUNCTION_NOT_ALLOWED; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.ON_ERROR_FUNC; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.ON_FILE_CHANGE_FUNC; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.ON_FILE_CSV_FUNC; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.ON_FILE_DELETE_FUNC; @@ -76,6 +77,7 @@ public void validate(SyntaxNodeAnalysisContext context) { FunctionDefinitionNode onFileChange = null; FunctionDefinitionNode onFileDelete = null; FunctionDefinitionNode onFileDeleted = null; + FunctionDefinitionNode onError = null; List contentMethods = new ArrayList<>(); List contentMethodNames = new ArrayList<>(); @@ -116,6 +118,9 @@ public void validate(SyntaxNodeAnalysisContext context) { contentMethods.add(functionDefinitionNode); contentMethodNames.add(funcName); break; + case ON_ERROR_FUNC: + onError = functionDefinitionNode; + break; default: // Invalid remote function name if (isRemoteFunction(context, functionDefinitionNode)) { @@ -169,5 +174,10 @@ public void validate(SyntaxNodeAnalysisContext context) { new FtpFileDeletedValidator(context, onFileDeleted, FtpFileDeletedValidator.DeletionMethodType.ON_FILE_DELETED).validate(); } + + // Validate error handler + if (onError != null) { + new FtpOnErrorValidator(context, onError).validate(); + } } } diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/PluginConstants.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/PluginConstants.java index 0041e77ea..bc67744ac 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/PluginConstants.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/PluginConstants.java @@ -42,6 +42,9 @@ private PluginConstants() {} public static final String ON_FILE_DELETED_FUNC = "onFileDeleted"; public static final String ON_FILE_DELETE_FUNC = "onFileDelete"; + // Error handler function name + public static final String ON_ERROR_FUNC = "onError"; + /** * All format-specific handler names. * These handlers automatically convert file content to typed data. @@ -61,6 +64,7 @@ private PluginConstants() {} // return types error or nil public static final String ERROR = "error"; + public static final String ERROR_PARAM = "Error"; // Code template related constants (Format-specific handlers) public static final String NODE_LOCATION = "node.location"; @@ -121,7 +125,14 @@ public enum CompilationErrors { ON_FILE_DELETED_DEPRECATED("'onFileDeleted' is deprecated and will be removed in a future release. " + "Use 'onFileDelete' instead.", "FTP_132"), BOTH_ON_FILE_DELETE_METHODS_NOT_ALLOWED("Cannot use both 'onFileDelete' and 'onFileDeleted' methods. " + - "Use only 'onFileDelete' as 'onFileDeleted' is deprecated.", "FTP_133"); + "Use only 'onFileDelete' as 'onFileDeleted' is deprecated.", "FTP_133"), + ON_ERROR_MUST_BE_REMOTE("onError method must be remote.", "FTP_134"), + INVALID_ON_ERROR_FIRST_PARAMETER("Invalid first parameter for onError. First parameter must be " + + "'ftp:Error' or 'error'.", "FTP_135"), + INVALID_ON_ERROR_SECOND_PARAMETER("Invalid second parameter for onError. " + + "Second parameter must be 'ftp:Caller'.", "FTP_136"), + TOO_MANY_PARAMETERS_ON_ERROR("Too many parameters for onError. Accepts at most 2 parameters: " + + "(error, caller?).", "FTP_137"); private final String error; private final String errorCode; diff --git a/spotbugs-exclude.xml b/spotbugs-exclude.xml index 97d9dcec4..096c785e1 100644 --- a/spotbugs-exclude.xml +++ b/spotbugs-exclude.xml @@ -28,4 +28,8 @@ + + + + From aebb5126699a5421ebe981b437503055fa203837 Mon Sep 17 00:00:00 2001 From: Niveathika Date: Thu, 5 Feb 2026 11:08:37 +0530 Subject: [PATCH 2/6] Implement onError routing --- ballerina/error.bal | 18 ++++++ .../stdlib/ftp/client/FtpClient.java | 8 ++- .../ftp/server/FormatMethodsHolder.java | 20 +++++++ .../ftp/server/FtpContentCallbackHandler.java | 55 ++++++++++++++++++- .../stdlib/ftp/util/FtpConstants.java | 4 ++ .../stdlib/ftp/util/FtpContentConverter.java | 51 ++++++++++------- .../io/ballerina/stdlib/ftp/util/FtpUtil.java | 54 ++++++++++++++++++ 7 files changed, 184 insertions(+), 26 deletions(-) diff --git a/ballerina/error.bal b/ballerina/error.bal index a2a1c976f..2fdc59cc5 100644 --- a/ballerina/error.bal +++ b/ballerina/error.bal @@ -37,6 +37,24 @@ public type InvalidConfigError distinct Error; # temporary file locks (450), or server-side processing errors (451). public type ServiceUnavailableError distinct Error; +# Represents an error that occurs when file content cannot be converted to the expected type. +# This includes JSON/XML parsing errors, CSV format errors, and record type binding failures. +# This error type is applicable to both Client operations and Listener callbacks. +# +# When used with the Listener, if an `onError` remote function is defined in the service, +# it will be invoked with this error type, allowing for custom error handling such as +# moving failed files to an error folder or sending notifications. +public type ContentBindingError distinct Error & error; + +# Detail record for ContentBindingError providing additional context about the binding failure. +# +# + filePath - The file path that caused the error +# + content - The raw file content as bytes that failed to bind +public type ContentBindingErrorDetail record {| + string filePath?; + byte[] content?; +|}; + # Represents an error that occurs when all retry attempts have been exhausted. # This error wraps the last failure encountered during retry attempts. public type AllRetryAttemptsFailedError distinct Error; diff --git a/native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClient.java b/native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClient.java index 895b2c25e..fbb9905a8 100644 --- a/native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClient.java +++ b/native/src/main/java/io/ballerina/stdlib/ftp/client/FtpClient.java @@ -393,7 +393,8 @@ public static Object getJson(Environment env, BObject clientConnector, BString f () -> { Object content = getAllContent(env, clientConnector, filePath); if (content instanceof byte[]) { - return convertBytesToJson((byte[]) content, typeDesc.getDescribingType(), laxDataBinding); + return convertBytesToJson((byte[]) content, typeDesc.getDescribingType(), laxDataBinding, + filePath.getValue()); } return content; }, @@ -409,7 +410,8 @@ public static Object getXml(Environment env, BObject clientConnector, BString fi () -> { Object content = getAllContent(env, clientConnector, filePath); if (content instanceof byte[]) { - return convertBytesToXml((byte[]) content, typeDesc.getDescribingType(), laxDataBinding); + return convertBytesToXml((byte[]) content, typeDesc.getDescribingType(), laxDataBinding, + filePath.getValue()); } return content; }, @@ -428,7 +430,7 @@ public static Object getCsv(Environment env, BObject clientConnector, BString fi Object content = getAllContent(env, clientConnector, filePath); if (content instanceof byte[]) { return convertBytesToCsv(env, (byte[]) content, typeDesc.getDescribingType(), - laxDataBinding, csvFailSafe, fileNamePrefix); + laxDataBinding, csvFailSafe, fileNamePrefix, filePath.getValue()); } return content; }, diff --git a/native/src/main/java/io/ballerina/stdlib/ftp/server/FormatMethodsHolder.java b/native/src/main/java/io/ballerina/stdlib/ftp/server/FormatMethodsHolder.java index 3dd2f5a0a..b0d106341 100644 --- a/native/src/main/java/io/ballerina/stdlib/ftp/server/FormatMethodsHolder.java +++ b/native/src/main/java/io/ballerina/stdlib/ftp/server/FormatMethodsHolder.java @@ -47,11 +47,13 @@ public class FormatMethodsHolder { private final BObject service; private final Map annotationPatternToMethod; private final Map availableContentMethods; + private final MethodType onErrorMethod; public FormatMethodsHolder(BObject service) { this.service = service; this.annotationPatternToMethod = new HashMap<>(); this.availableContentMethods = new HashMap<>(); + this.onErrorMethod = FtpUtil.getOnErrorMethod(service).orElse(null); initializeMethodMappings(); } @@ -175,4 +177,22 @@ private Optional> getFileConfigAnnotation(MethodType metho public boolean hasContentMethods() { return !availableContentMethods.isEmpty(); } + + /** + * Gets the onError method if it exists in the service. + * + * @return Optional containing the onError MethodType + */ + public Optional getOnErrorMethod() { + return Optional.ofNullable(onErrorMethod); + } + + /** + * Checks if an onError handler is available. + * + * @return true if onError method is available + */ + public boolean hasOnErrorMethod() { + return onErrorMethod != null; + } } diff --git a/native/src/main/java/io/ballerina/stdlib/ftp/server/FtpContentCallbackHandler.java b/native/src/main/java/io/ballerina/stdlib/ftp/server/FtpContentCallbackHandler.java index a013a5eb3..2fb37fb8c 100644 --- a/native/src/main/java/io/ballerina/stdlib/ftp/server/FtpContentCallbackHandler.java +++ b/native/src/main/java/io/ballerina/stdlib/ftp/server/FtpContentCallbackHandler.java @@ -117,6 +117,13 @@ public void processContentCallbacks(Environment env, BObject service, RemoteFile MethodType methodType = methodTypeOpt.get(); Object convertedContent = convertFileContent(env, fileObject, inputStream, methodType); + // 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); + continue; + } + // Prepare method arguments Object[] methodArguments = prepareContentMethodArguments(methodType, convertedContent, fileInfo, callerObject); @@ -185,13 +192,16 @@ private Object convertFileContent(Environment environment, FileObject fileObject } else { byte[] fileContent = fetchAllFileContentFromRemote(fileObject, inputStream); String fileNamePrefix = deriveFileNamePrefix(fileObject); + String filePath = fileObject.getName().getURI(); return switch (methodName) { case ON_FILE_REMOTE_FUNCTION -> convertToBallerinaByteArray(fileContent); case ON_FILE_TEXT_REMOTE_FUNCTION -> convertBytesToString(fileContent); - case ON_FILE_JSON_REMOTE_FUNCTION -> convertBytesToJson(fileContent, firstParamType, laxDataBinding); - case ON_FILE_XML_REMOTE_FUNCTION -> convertBytesToXml(fileContent, firstParamType, laxDataBinding); + case ON_FILE_JSON_REMOTE_FUNCTION -> convertBytesToJson(fileContent, firstParamType, laxDataBinding, + filePath); + case ON_FILE_XML_REMOTE_FUNCTION -> convertBytesToXml(fileContent, firstParamType, laxDataBinding, + filePath); case ON_FILE_CSV_REMOTE_FUNCTION -> convertBytesToCsv(environment, fileContent, firstParamType, - laxDataBinding, csvFailSafe, fileNamePrefix); + laxDataBinding, csvFailSafe, fileNamePrefix, filePath); default -> throw new IllegalArgumentException("Unknown content method: " + methodName); }; } @@ -267,6 +277,45 @@ private BMap createFileInfoRecord(FileInfo fileInfo) { ); } + /** + * Routes error to onError handler if available. + */ + private void routeToOnError(BObject service, FormatMethodsHolder holder, BError error, BObject callerObject) { + if (!holder.hasOnErrorMethod()) { + // No onError handler, error is already logged + return; + } + + Optional onErrorMethodOpt = holder.getOnErrorMethod(); + if (onErrorMethodOpt.isEmpty()) { + return; + } + + MethodType onErrorMethod = onErrorMethodOpt.get(); + + // Prepare arguments for onError method + Object[] methodArguments = prepareOnErrorMethodArguments(onErrorMethod, error, callerObject); + + // Invoke onError asynchronously + invokeContentMethodAsync(service, onErrorMethod.getName(), methodArguments); + } + + /** + * Prepares method arguments for onError handler. + * onError accepts: (error) or (error, caller) + */ + private Object[] prepareOnErrorMethodArguments(MethodType methodType, BError error, BObject callerObject) { + Parameter[] parameters = methodType.getParameters(); + + if (parameters.length == 2) { + // Two parameters: error, Caller + return new Object[]{error, callerObject}; + } + + // Default: only error + return new Object[]{error}; + } + /** * Invokes the content handler method asynchronously. */ diff --git a/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpConstants.java b/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpConstants.java index 7fc507fd6..fb4e18b58 100644 --- a/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpConstants.java +++ b/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpConstants.java @@ -173,6 +173,10 @@ private FtpConstants() { public static final String ON_FILE_CSV_REMOTE_FUNCTION = "onFileCsv"; public static final String ON_FILE_DELETE_REMOTE_FUNCTION = "onFileDelete"; public static final String ON_FILE_DELETED_REMOTE_FUNCTION = "onFileDeleted"; + public static final String ON_ERROR_REMOTE_FUNCTION = "onError"; + + // Error types + public static final String CONTENT_BINDING_ERROR = "ContentBindingError"; public static final String APACHE_VFS2_PACKAGE_NAME = "org.apache.commons.vfs2"; public static final String BALLERINA_FTP_PACKAGE_NAME = "io.ballerina.stdlib.ftp"; diff --git a/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpContentConverter.java b/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpContentConverter.java index a2c7c2b02..526b38ac7 100644 --- a/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpContentConverter.java +++ b/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpContentConverter.java @@ -40,8 +40,6 @@ import java.nio.charset.StandardCharsets; import static io.ballerina.lib.data.csvdata.csv.Native.parseBytes; -import static io.ballerina.stdlib.ftp.util.FtpConstants.FTP_ERROR; -import static io.ballerina.stdlib.ftp.util.FtpUtil.ErrorType.Error; /** * Utility class for converting file content to various Ballerina types using data binding modules. @@ -83,23 +81,27 @@ public static BString convertBytesToString(byte[] content) { * * @param content The byte array content * @param targetType The target Ballerina type for data binding - * @return Ballerina JSON object or BError + * @param laxDataBinding Whether to allow lax data binding + * @param filePath The file path for error reporting + * @return Ballerina JSON object or ContentBindingError */ - public static Object convertBytesToJson(byte[] content, Type targetType, boolean laxDataBinding) { + public static Object convertBytesToJson(byte[] content, Type targetType, boolean laxDataBinding, String filePath) { try { BArray byteArray = ValueCreator.createArrayValue(content); BMap options = createJsonParseOptions(laxDataBinding); BTypedesc typedesc = ValueCreator.createTypedescValue(targetType); Object result = io.ballerina.lib.data.jsondata.json.Native.parseBytes(byteArray, options, typedesc); - if (result instanceof BError) { - log.error("Failed to parse JSON content: {}", ((BError) result).getMessage()); - return FtpUtil.createError(((BError) result).getErrorMessage().getValue(), FTP_ERROR); + 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.createError("Failed to parse JSON content: " + e.getMessage(), Error.errorType()); + return FtpUtil.createContentBindingError("Failed to parse JSON content: " + e.getMessage(), e, + filePath, content); } } @@ -108,9 +110,11 @@ public static Object convertBytesToJson(byte[] content, Type targetType, boolean * * @param content The byte array content * @param targetType The target Ballerina type for data binding - * @return Ballerina XML object or BError + * @param laxDataBinding Whether to allow lax data binding + * @param filePath The file path for error reporting + * @return Ballerina XML object or ContentBindingError */ - public static Object convertBytesToXml(byte[] content, Type targetType, boolean laxDataBinding) { + public static Object convertBytesToXml(byte[] content, Type targetType, boolean laxDataBinding, String filePath) { try { if (targetType.getQualifiedName().equals("xml")) { return XmlUtils.parse(StringUtils.fromString(new String(content, StandardCharsets.UTF_8))); @@ -119,24 +123,30 @@ public static Object convertBytesToXml(byte[] content, Type targetType, boolean BMap mapValue = createXmlParseOptions(laxDataBinding); Object bXml = Native.parseBytes( ValueCreator.createArrayValue(content), mapValue, ValueCreator.createTypedescValue(targetType)); - if (bXml instanceof BError) { - return FtpUtil.createError(((BError) bXml).getErrorMessage().getValue(), FTP_ERROR); + if (bXml instanceof BError bError) { + return FtpUtil.createContentBindingError(bError.getErrorMessage().getValue(), bError, filePath, + content); } return bXml; } catch (BError e) { - return FtpUtil.createError(e.getErrorMessage().getValue(), FTP_ERROR); + return FtpUtil.createContentBindingError(e.getErrorMessage().getValue(), e, filePath, content); } } /** * Converts byte array to CSV using data.csvdata module. * + * @param env The Ballerina environment * @param content The byte array content * @param targetType The target Ballerina type for data binding - * @return Ballerina CSV data (string[][], record[][], or custom type) or BError + * @param laxDataBinding Whether to allow lax data binding + * @param csvFailSafeConfigs CSV fail-safe configuration + * @param fileNamePrefix The file name prefix for error log + * @param filePath The file path for error reporting + * @return Ballerina CSV data (string[][], record[][], or custom type) or ContentBindingError */ public static Object convertBytesToCsv(Environment env, byte[] content, Type targetType, boolean laxDataBinding, - BMap csvFailSafeConfigs, String fileNamePrefix) { + BMap csvFailSafeConfigs, String fileNamePrefix, String filePath) { try { BArray byteArray = ValueCreator.createArrayValue(content); BMap options = createCsvParseOptions(laxDataBinding, csvFailSafeConfigs, fileNamePrefix); @@ -146,16 +156,17 @@ public static Object convertBytesToCsv(Environment env, byte[] content, Type tar Object result = parseBytes(env, byteArray, options, typedesc); - if (result instanceof BError) { - log.error("Failed to parse CSV content: {}", ((BError) result).getMessage()); - return FtpUtil.createError("Failed to parse CSV content: " + ((BError) result).getErrorMessage(), - Error.errorType()); + 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.createError("Failed to parse CSV content: " + e.getMessage(), Error.errorType()); + return FtpUtil.createContentBindingError("Failed to parse CSV content: " + e.getMessage(), e, + filePath, content); } } diff --git a/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java b/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java index f5a32b565..6cff67e22 100644 --- a/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java @@ -594,6 +594,59 @@ public static Optional getOnFileDeletedMethod(BObject service) { .findFirst(); } + /** + * Gets the onError method from a service if it exists. + * + * @param service The BObject service + * @return Optional containing the MethodType if onError method exists + */ + public static Optional getOnErrorMethod(BObject service) { + MethodType[] methodTypes = ((ObjectType) TypeUtils.getReferredType(TypeUtils.getType(service))).getMethods(); + return Stream.of(methodTypes) + .filter(methodType -> FtpConstants.ON_ERROR_REMOTE_FUNCTION.equals(methodType.getName())) + .findFirst(); + } + + /** + * Creates a ContentBindingError with the given message and cause. + * + * @param message The error message + * @param cause The underlying cause (can be null) + * @param filePath The path of the file that caused the error + * @param content The raw file content as bytes that failed to bind (can be null) + * @return A ContentBindingError BError + */ + public static BError createContentBindingError(String message, Throwable cause, String filePath, + byte[] content) { + Map detailMap = new HashMap<>(); + if (filePath != null) { + detailMap.put("filePath", StringUtils.fromString(filePath)); + } + if (content != null) { + detailMap.put("content", ValueCreator.createArrayValue(content)); + } + + BMap detailRecord = ValueCreator.createRecordValue( + new Module(FtpConstants.FTP_ORG_NAME, FtpConstants.FTP_MODULE_NAME, getFtpPackage().getMajorVersion()), + "ContentBindingErrorDetail", + detailMap + ); + + String safeMessage = maskUrlPassword(message); + BError causeError = null; + if (cause != null) { + if (cause instanceof BError) { + causeError = (BError) cause; + } else { + String causeMsg = cause.getMessage() != null ? cause.getMessage() : "Unknown error"; + String safeCauseMsg = maskUrlPassword(causeMsg); + causeError = ErrorCreator.createError(StringUtils.fromString(safeCauseMsg)); + } + } + return ErrorCreator.createError(ModuleUtils.getModule(), ErrorType.ContentBindingError.errorType(), + StringUtils.fromString(safeMessage), causeError, detailRecord); + } + public static String getAuthMethod(Object authMethodObj) { return authMethodObj.toString().toLowerCase().replace("_", "-"); } @@ -624,6 +677,7 @@ public enum ErrorType { FileAlreadyExistsError("FileAlreadyExistsError"), InvalidConfigError("InvalidConfigError"), ServiceUnavailableError("ServiceUnavailableError"), + ContentBindingError("ContentBindingError"), AllRetryAttemptsFailedError("AllRetryAttemptsFailedError"); private String errorType; From 90016fca2c73ab3d7564e477e4c19d9f75820513 Mon Sep 17 00:00:00 2001 From: Niveathika Date: Thu, 5 Feb 2026 11:52:22 +0530 Subject: [PATCH 3/6] Add testcases --- ballerina/tests/client_endpoint_test.bal | 82 +++++- ballerina/tests/listener_on_error_test.bal | 240 ++++++++++++++++++ .../mockServerUtils/MockFtpServer.java | 2 + 3 files changed, 323 insertions(+), 1 deletion(-) create mode 100644 ballerina/tests/listener_on_error_test.bal diff --git a/ballerina/tests/client_endpoint_test.bal b/ballerina/tests/client_endpoint_test.bal index c175cfbe4..ff86250e7 100644 --- a/ballerina/tests/client_endpoint_test.bal +++ b/ballerina/tests/client_endpoint_test.bal @@ -323,6 +323,85 @@ function testPutXmlFailure() returns error? { test:assertTrue(got is Error, msg = "XML content binding should have failed for non-XML content"); } +// Test that getJson returns ContentBindingError when JSON parsing fails +@test:Config {dependsOn: [testPutFileContent]} +function testGetJsonContentBindingError() returns error? { + string path = "/home/in/invalid-json.txt"; + // Write invalid JSON content + check (clientEp)->putText(path, "this is not valid json {{{"); + + json|Error result = (clientEp)->getJson(path); + test:assertTrue(result is ContentBindingError, msg = "getJson should return ContentBindingError for invalid JSON"); + if result is ContentBindingError { + test:assertTrue(result.detail().filePath is string, msg = "ContentBindingError should contain filePath"); + test:assertTrue(result.detail().content is byte[], msg = "ContentBindingError should contain content bytes"); + } + + // Cleanup + check (clientEp)->delete(path); +} + +// Test that getXml returns ContentBindingError when XML parsing fails +@test:Config {dependsOn: [testPutFileContent]} +function testGetXmlContentBindingError() returns error? { + string path = "/home/in/invalid-xml.txt"; + // Write invalid XML content + check (clientEp)->putText(path, "this is not valid xml <<<>>>"); + + xml|Error result = (clientEp)->getXml(path); + test:assertTrue(result is ContentBindingError, msg = "getXml should return ContentBindingError for invalid XML"); + if result is ContentBindingError { + test:assertTrue(result.detail().filePath is string, msg = "ContentBindingError should contain filePath"); + test:assertTrue(result.detail().content is byte[], msg = "ContentBindingError should contain content bytes"); + } + + // Cleanup + check (clientEp)->delete(path); +} + +// Test that getJson with typed binding returns ContentBindingError when type doesn't match +@test:Config {dependsOn: [testPutFileContent]} +function testGetJsonTypedContentBindingError() returns error? { + string path = "/home/in/json-typed-binding-error.json"; + // Write JSON missing required field 'age' + json content = {name: "Alice"}; + check (clientEp)->putJson(path, content); + + // Try to bind to a record that requires 'age' field + PersonStrict|Error result = (clientEp)->getJson(path); + test:assertTrue(result is ContentBindingError, msg = "getJson should return ContentBindingError when type binding fails"); + if result is ContentBindingError { + test:assertTrue(result.detail().filePath is string, msg = "ContentBindingError should contain filePath"); + test:assertTrue(result.detail().content is byte[], msg = "ContentBindingError should contain content bytes"); + } + + // Cleanup + check (clientEp)->delete(path); +} + +// Test that getCsv returns ContentBindingError when type binding fails +@test:Config {dependsOn: [testPutFileContent]} +function testGetCsvContentBindingError() returns error? { + string path = "/home/in/csv-binding-error.csv"; + // Write CSV with non-integer value in age column + string[][] csvData = [ + ["name", "age"], + ["Alice", "not-a-number"] // Invalid: age should be int + ]; + check (clientEp)->putCsv(path, csvData); + + // Try to bind to a record that expects 'age' as int + CsvPersonStrict[]|Error result = (clientEp)->getCsv(path); + test:assertTrue(result is ContentBindingError, msg = "getCsv should return ContentBindingError when type binding fails"); + if result is ContentBindingError { + test:assertTrue(result.detail().filePath is string, msg = "ContentBindingError should contain filePath"); + test:assertTrue(result.detail().content is byte[], msg = "ContentBindingError should contain content bytes"); + } + + // Cleanup + check (clientEp)->delete(path); +} + @test:Config {dependsOn: [testPutFileContent]} function testPutText() returns error? { string txt = "hello text content"; @@ -1133,6 +1212,7 @@ public function testListFiles() { "childDirectory", "delete", "test2.txt", + "onerror-tests", "test4.txt", "child_directory", "content-methods", @@ -1140,7 +1220,7 @@ public function testListFiles() { "retry", "test3.txt" ]; - int[] fileSizes = [0, 61, 0, 0, 0, 0, 0, 145, 0, 0, 16400, 9000, 0, 0, 0, 0, 12]; + int[] fileSizes = [0, 61, 0, 0, 0, 0, 0, 145, 0, 0, 16400, 0, 9000, 0, 0, 0, 0, 12]; FileInfo[]|Error response = (clientEp)->list("/home/in"); if response is FileInfo[] { log:printInfo("List of files/directories: "); diff --git a/ballerina/tests/listener_on_error_test.bal b/ballerina/tests/listener_on_error_test.bal new file mode 100644 index 000000000..8b3573bfd --- /dev/null +++ b/ballerina/tests/listener_on_error_test.bal @@ -0,0 +1,240 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/lang.runtime; +import ballerina/log; +import ballerina/test; + +// Record type for testing JSON binding errors +type StrictPerson record {| + string name; + int age; + string email; // Required field that's missing in test JSON +|}; + +// Global tracking variables for onError tests +boolean onErrorInvoked = false; +ContentBindingError? lastBindingError = (); +string lastErrorFilePath = ""; +int onErrorInvocationCount = 0; + +// Directory for onError tests +const string ON_ERROR_TEST_DIR = "/home/in/onerror-tests"; + +@test:Config { + enable: true +} +public function testOnErrorBasic() returns error? { + // Reset state + onErrorInvoked = false; + lastBindingError = (); + lastErrorFilePath = ""; + onErrorInvocationCount = 0; + + // Service with onFileJson that will fail and onError handler + Service onErrorService = service object { + remote function onFileJson(StrictPerson content, FileInfo fileInfo, Caller caller) returns error? { + // This should not be called since binding will fail + log:printInfo(string `onFileJson invoked for: ${fileInfo.name}`); + } + + remote function onError(Error err, Caller caller) returns error? { + log:printInfo("onError invoked"); + log:printError("Binding error", err); + onErrorInvoked = true; + // Verify that the error is a ContentBindingError + if err is ContentBindingError { + lastBindingError = err; + // Access filePath from error detail + lastErrorFilePath = err.detail().filePath ?: ""; + } + onErrorInvocationCount += 1; + } + }; + + // Create listener + Listener onErrorListener = check new ({ + protocol: FTP, + host: "127.0.0.1", + auth: {credentials: {username: "wso2", password: "wso2123"}}, + port: 21212, + path: ON_ERROR_TEST_DIR, + pollingInterval: 4, + fileNamePattern: "errortest.*\\.json" + }); + + check onErrorListener.attach(onErrorService); + check onErrorListener.'start(); + runtime:registerListener(onErrorListener); + + // Upload invalid JSON file (missing required field 'email') + check (clientEp)->putText(ON_ERROR_TEST_DIR + "/errortest.json", + "{\"name\": \"John\", \"age\": 30}"); + runtime:sleep(10); + + // Cleanup + runtime:deregisterListener(onErrorListener); + check onErrorListener.gracefulStop(); + + test:assertTrue(onErrorInvoked, "onError should have been invoked"); + test:assertTrue(lastBindingError is ContentBindingError, "Should have received ContentBindingError"); + test:assertTrue(lastErrorFilePath.endsWith(".json"), "Error file path should end with .json"); +} + +@test:Config { + enable: true, + dependsOn: [testOnErrorBasic] +} +public function testOnErrorWithMinimalParameters() returns error? { + // Reset state + onErrorInvoked = false; + lastBindingError = (); + onErrorInvocationCount = 0; + + // Service with onError handler with only error parameter + Service minimalOnErrorService = service object { + remote function onFileJson(StrictPerson content, FileInfo fileInfo) returns error? { + // This should not be called since binding will fail + log:printInfo(string `onFileJson invoked for: ${fileInfo.name}`); + } + + remote function onError(Error err) returns error? { + log:printInfo("onError (minimal) invoked"); + log:printError("Binding error", err); + onErrorInvoked = true; + // Verify that the error is a ContentBindingError + if err is ContentBindingError { + lastBindingError = err; + } + onErrorInvocationCount += 1; + } + }; + + // Create listener + Listener minimalListener = check new ({ + protocol: FTP, + host: "127.0.0.1", + auth: {credentials: {username: "wso2", password: "wso2123"}}, + port: 21212, + path: ON_ERROR_TEST_DIR, + pollingInterval: 4, + fileNamePattern: "minimal.*\\.json" + }); + + check minimalListener.attach(minimalOnErrorService); + check minimalListener.'start(); + runtime:registerListener(minimalListener); + + // Upload invalid JSON + check (clientEp)->putText(ON_ERROR_TEST_DIR + "/minimal.json", + "{\"name\": \"Jane\", \"age\": 25}"); + runtime:sleep(10); + + // Cleanup + runtime:deregisterListener(minimalListener); + check minimalListener.gracefulStop(); + + test:assertTrue(onErrorInvoked, "onError (minimal) should have been invoked"); + test:assertTrue(lastBindingError is ContentBindingError, "Should have received ContentBindingError"); +} + +@test:Config { + enable: true, + dependsOn: [testOnErrorWithMinimalParameters] +} +public function testOnErrorWithCallerOperations() returns error? { + // Reset state + onErrorInvoked = false; + lastBindingError = (); + lastErrorFilePath = ""; + onErrorInvocationCount = 0; + + // Service that uses caller to perform operations on failed files + Service moveOnErrorService = service object { + remote function onFileJson(StrictPerson content, FileInfo fileInfo, Caller caller) returns error? { + log:printInfo(string `onFileJson invoked for: ${fileInfo.name}`); + } + + remote function onError(Error err, Caller caller) returns error? { + onErrorInvoked = true; + onErrorInvocationCount += 1; + + // Verify that the error is a ContentBindingError and access details + if err is ContentBindingError { + string? filePath = err.detail().filePath; + log:printInfo(string `onError invoked for file: ${filePath ?: "unknown"}`); + lastBindingError = err; + lastErrorFilePath = filePath ?: ""; + + // Try to delete the problematic file using caller + if filePath is string { + Error? deleteResult = caller->delete(filePath); + if deleteResult is () { + log:printInfo("Successfully deleted error file"); + } else { + log:printError("Failed to delete error file", deleteResult); + } + } + } else { + log:printError("Unexpected error type", err); + } + } + }; + + // Create listener + Listener moveListener = check new ({ + protocol: FTP, + host: "127.0.0.1", + auth: {credentials: {username: "wso2", password: "wso2123"}}, + port: 21212, + path: ON_ERROR_TEST_DIR, + pollingInterval: 4, + fileNamePattern: "moveerror.*\\.json" + }); + + check moveListener.attach(moveOnErrorService); + check moveListener.'start(); + runtime:registerListener(moveListener); + + // Upload invalid JSON + check (clientEp)->putText(ON_ERROR_TEST_DIR + "/moveerror.json", + "{\"name\": \"Bob\", \"age\": 40}"); + runtime:sleep(10); + + // Cleanup + runtime:deregisterListener(moveListener); + check moveListener.gracefulStop(); + + test:assertTrue(onErrorInvoked, "onError should have been invoked"); + test:assertTrue(lastBindingError is ContentBindingError, "Should have received ContentBindingError"); +} + +@test:Config { + enable: true, + dependsOn: [testOnErrorWithCallerOperations] +} +public function testContentBindingErrorType() returns error? { + // Test that ContentBindingError is a distinct error type with detail record + ContentBindingError testError = error ContentBindingError("Test binding error", + filePath = "/test/file.json", + content = [72, 101, 108, 108, 111] // "Hello" as bytes + ); + + test:assertTrue(testError is Error, "ContentBindingError should be a subtype of Error"); + test:assertEquals(testError.message(), "Test binding error", "Error message should match"); + test:assertEquals(testError.detail().filePath, "/test/file.json", "filePath should match"); + test:assertEquals(testError.detail().content, [72, 101, 108, 108, 111], "content should match"); +} diff --git a/test-utils/src/main/java/io/ballerina/stdlib/ftp/testutils/mockServerUtils/MockFtpServer.java b/test-utils/src/main/java/io/ballerina/stdlib/ftp/testutils/mockServerUtils/MockFtpServer.java index c85143cc7..ddad50341 100644 --- a/test-utils/src/main/java/io/ballerina/stdlib/ftp/testutils/mockServerUtils/MockFtpServer.java +++ b/test-utils/src/main/java/io/ballerina/stdlib/ftp/testutils/mockServerUtils/MockFtpServer.java @@ -123,6 +123,8 @@ public static Object initFtpServer() throws Exception { fileSystem.add(new FileEntry("/home/in/child_directory/content1.txt")); fileSystem.add(new FileEntry("/home/in/child_directory/content2.txt")); + fileSystem.add(new DirectoryEntry("/home/in/onerror-tests")); + fileSystem.add(new DirectoryEntry("/home/in/delete")); fileSystem.add(new FileEntry("/home/in/delete/.init", "")); From e1a707553b16e94dcce18c72e28b560f60f0225f Mon Sep 17 00:00:00 2001 From: Niveathika Date: Thu, 5 Feb 2026 11:52:09 +0530 Subject: [PATCH 4/6] Add docs --- changelog.md | 1 + docs/spec/spec.md | 56 ++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/changelog.md b/changelog.md index 8995364f4..2ed369ff1 100644 --- a/changelog.md +++ b/changelog.md @@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## unreleased ### Added +- [Introduce onError remote function](https://github.com/ballerina-platform/ballerina-library/issues/8605) - [Add automatic retry support with exponential backoff for FTP client](https://github.com/ballerina-platform/ballerina-library/issues/8585) - [Add FTP Listener Coordination Support](https://github.com/ballerina-platform/ballerina-library/issues/8490) - [Add distinct error types](https://github.com/ballerina-platform/ballerina-library/issues/8597) diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 50fa64433..44cdaa651 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -3,7 +3,7 @@ _Owners_: @shafreenAnfar @dilanSachi @Bhashinee _Reviewers_: @shafreenAnfar @Bhashinee _Created_: 2020/10/28 -_Updated_: 2026/01/26 +_Updated_: 2026/02/05 _Edition_: Swan Lake ## Introduction @@ -187,6 +187,19 @@ public type InvalidConfigError distinct Error; public type ServiceUnavailableError distinct Error; ``` +* `ContentBindingError` - Represents errors that occur when file content cannot be converted to the expected type. This includes JSON/XML parsing errors, CSV format errors, and record type binding failures. This error type is applicable to both Client operations and Listener callbacks. When used with the Listener, if an `onError` remote function is defined in the service, it will be invoked with this error type. +```ballerina +public type ContentBindingError distinct Error; +``` + +The `ContentBindingError` includes a detail record providing additional context: +```ballerina +public type ContentBindingErrorDetail record {| + string filePath?; // The file path that caused the error + byte[] content?; // The raw file content as bytes that failed to bind +|}; +``` + All specific error types are subtypes of the base `Error` type, allowing for both specific and general error handling: ```ballerina // Handle specific error types @@ -480,7 +493,7 @@ remote isolated function getBytes(string path) returns byte[]|Error; # + return - File content as text, or an error if the operation fails remote isolated function getText(string path) returns string|Error; ``` -* `getJson()` can be used to read a file as JSON data. +* `getJson()` can be used to read a file as JSON data. Returns `ContentBindingError` if the file content cannot be parsed as JSON or bound to the target type. ```ballerina # Read a file as JSON data. # ```ballerina @@ -489,10 +502,10 @@ remote isolated function getText(string path) returns string|Error; # # + path - Location of the file on the server # + targetType - What format should the data have? (JSON, structured data, or a custom format) -# + return - The file content as JSON or an error if the operation fails +# + return - The file content as JSON, or `ContentBindingError` if parsing/binding fails, or other `Error` for connection issues remote isolated function getJson(string path, typedesc targetType = <>) returns targetType|Error; ``` -* `getXml()` can be used to read a file as XML data. +* `getXml()` can be used to read a file as XML data. Returns `ContentBindingError` if the file content cannot be parsed as XML or bound to the target type. ```ballerina # Read a file as XML data. # ```ballerina @@ -501,10 +514,10 @@ remote isolated function getJson(string path, typedesc targetTyp # # + path - Location of the file on the server # + targetType - What format should the data have? (XML, structured data, or a custom format) -# + return - The file content as XML or an error if the operation fails +# + return - The file content as XML, or `ContentBindingError` if parsing/binding fails, or other `Error` for connection issues remote isolated function getXml(string path, typedesc targetType = <>) returns targetType|Error; ``` -* `getCsv()` can be used to read a CSV file from the server. +* `getCsv()` can be used to read a CSV file from the server. Returns `ContentBindingError` if the file content cannot be parsed or bound to the target type. ```ballerina # Read a CSV (comma-separated) file from the server. # The first row of the CSV file should contain column names (headers). @@ -514,7 +527,7 @@ remote isolated function getXml(string path, typedesc targetType # # + path - Location of the CSV file on the server # + targetType - What format should the data have? (Table or structured records) -# + return - The CSV file content as a table or records, or an error if the operation fails +# + return - The CSV file content as a table or records, or `ContentBindingError` if parsing/binding fails, or other `Error` for connection issues remote isolated function getCsv(string path, typedesc targetType = <>) returns targetType|Error; ``` * `getBytesAsStream()` can be used to read file content as a stream of byte chunks. @@ -851,7 +864,34 @@ remote function onFileDelete(string deletedFile, ftp:Caller caller) returns erro } ``` -**Optional parameters:** The `fileInfo` and `caller` parameters can be omitted if not needed in your implementation. +* `onError()` - Triggered when a content binding error occurs during file processing. This allows centralized error handling for data binding failures: +```ballerina +remote function onError(ftp:Error err, ftp:Caller caller) returns error? { + // Check if it's a content binding error to access details + if err is ftp:ContentBindingError { + string? filePath = err.detail().filePath; + byte[]? content = err.detail().content; + + log:printError("Failed to process file: " + (filePath ?: "unknown"), err); + if filePath is string { + check caller->move(filePath, "/error/failed_file"); + } + } +} +``` + +The `onError` handler receives: +- `err`: The base `ftp:Error` type. For content binding failures, this will be a `ContentBindingError`. Users should type-check with `if err is ftp:ContentBindingError` to access the detail record containing `filePath` and `content`. +- `caller` (optional): FTP caller for performing recovery operations + +**Supported parameter signatures:** +- `onError(error err)` - Using Ballerina's error type descriptor +- `onError(ftp:Error err)` - Using the FTP module's Error type +- `onError(ftp:Error err, ftp:Caller caller)` - With caller for recovery operations + +If `onError` is not defined, binding errors are logged and the file is skipped. + +**Optional parameters:** The `caller` parameter can be omitted if not needed in your implementation. All format-specific callbacks receive `fileInfo` (metadata about the file) and optionally `caller` (to perform additional FTP operations). The data is automatically parsed based on the callback type. From 107ffe1b7129fd4d70448efdaa0a0a72a8320191 Mon Sep 17 00:00:00 2001 From: Niveathika Date: Thu, 5 Feb 2026 15:04:36 +0530 Subject: [PATCH 5/6] Enable logging in tests --- compiler-plugin-tests/build.gradle | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler-plugin-tests/build.gradle b/compiler-plugin-tests/build.gradle index e2a057a99..ed29d0280 100644 --- a/compiler-plugin-tests/build.gradle +++ b/compiler-plugin-tests/build.gradle @@ -102,6 +102,10 @@ test { useTestNG() { suites 'src/test/resources/testng.xml' } + testLogging { + showStandardStreams = true + events "passed", "failed", "skipped", "standardOut", "standardError" + } } test.dependsOn ":ftp-ballerina:build" From f61b96121fab14527bb4c3cb999529cf7d399d2c Mon Sep 17 00:00:00 2001 From: Niveathika Date: Thu, 5 Feb 2026 15:16:20 +0530 Subject: [PATCH 6/6] Improve error type validation --- .../ftp/plugin/FtpServiceValidationTest.java | 8 +++ .../valid_on_error_service_4/Ballerina.toml | 10 +++ .../valid_on_error_service_4/service.bal | 37 +++++++++++ .../ftp/plugin/FtpOnErrorValidator.java | 66 ++++++++++++++----- 4 files changed, 103 insertions(+), 18 deletions(-) create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/service.bal diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java index 2bd342729..3ed0bc849 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java @@ -759,6 +759,14 @@ public void testValidOnErrorService3() { Assert.assertEquals(diagnosticResult.errors().size(), 0); } + @Test(description = "Validation with valid onError handler (ftp:ContentBindingError subtype)") + public void testValidOnErrorService4() { + Package currentPackage = loadPackage("valid_on_error_service_4"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errors().size(), 0); + } + @Test(description = "Validation when onError method is not remote") public void testInvalidOnErrorService1() { Package currentPackage = loadPackage("invalid_on_error_service_1"); diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/Ballerina.toml new file mode 100644 index 000000000..11386db25 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/Ballerina.toml @@ -0,0 +1,10 @@ +[package] +org = "ftp_test" +name = "valid_on_error_service_4" +version = "0.1.0" + +[[dependency]] +org="ballerina" +name="ftp" +version="2.16.1" +repository="local" \ No newline at end of file diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/service.bal new file mode 100644 index 000000000..abc330081 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/valid_on_error_service_4/service.bal @@ -0,0 +1,37 @@ +// Copyright (c) 2026 WSO2 LLC. (http://www.wso2.com) All Rights Reserved. +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/ftp; + +listener ftp:Listener remoteServer = check new ({ + protocol: ftp:FTP, + host: "localhost", + port: 21213, + pollingInterval: 2, + path: "/upload/", + fileNamePattern: "(.*)" +}); + +// Valid service with onError handler (ftp:ContentBindingError subtype) +service "OnErrorService" on remoteServer { + remote function onFileJson(json content, ftp:FileInfo fileInfo) returns error? { + return; + } + + remote function onError(ftp:ContentBindingError err) returns error? { + return; + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java index b008cfa15..6e91d153f 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpOnErrorValidator.java @@ -20,19 +20,20 @@ import io.ballerina.compiler.api.SemanticModel; import io.ballerina.compiler.api.symbols.Symbol; +import io.ballerina.compiler.api.symbols.TypeDefinitionSymbol; +import io.ballerina.compiler.api.symbols.TypeDescKind; +import io.ballerina.compiler.api.symbols.TypeReferenceTypeSymbol; +import io.ballerina.compiler.api.symbols.TypeSymbol; import io.ballerina.compiler.syntax.tree.FunctionDefinitionNode; -import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.ParameterNode; -import io.ballerina.compiler.syntax.tree.RequiredParameterNode; import io.ballerina.compiler.syntax.tree.SeparatedNodeList; -import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; import io.ballerina.tools.diagnostics.DiagnosticSeverity; import java.util.Optional; -import static io.ballerina.compiler.syntax.tree.SyntaxKind.ERROR_TYPE_DESC; -import static io.ballerina.compiler.syntax.tree.SyntaxKind.QUALIFIED_NAME_REFERENCE; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.PACKAGE_ORG; +import static io.ballerina.stdlib.ftp.plugin.PluginConstants.PACKAGE_PREFIX; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.ERROR_PARAM; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.INVALID_ON_ERROR_FIRST_PARAMETER; import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.INVALID_ON_ERROR_SECOND_PARAMETER; @@ -40,7 +41,6 @@ import static io.ballerina.stdlib.ftp.plugin.PluginConstants.CompilationErrors.TOO_MANY_PARAMETERS_ON_ERROR; import static io.ballerina.stdlib.ftp.plugin.PluginUtils.getDiagnostic; import static io.ballerina.stdlib.ftp.plugin.PluginUtils.isRemoteFunction; -import static io.ballerina.stdlib.ftp.plugin.PluginUtils.validateModuleId; /** * Validates the onError remote function in FTP service. @@ -97,22 +97,52 @@ public void validate() { } private void validateErrorParameter(ParameterNode parameterNode) { - SyntaxKind paramSyntaxKind = ((RequiredParameterNode) parameterNode).typeName().kind(); + Optional paramType = PluginUtils.getParameterTypeSymbol(parameterNode, context); + if (paramType.isEmpty()) { + context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_FIRST_PARAMETER, + DiagnosticSeverity.ERROR, parameterNode.location())); + return; + } + SemanticModel semanticModel = context.semanticModel(); - if (paramSyntaxKind.equals(QUALIFIED_NAME_REFERENCE)) { - Node parameterTypeNode = ((RequiredParameterNode) parameterNode).typeName(); - Optional paramSymbol = semanticModel.symbol(parameterTypeNode); - if (paramSymbol.isEmpty() || paramSymbol.get().getName().isEmpty() || - !paramSymbol.get().getName().get().equals(ERROR_PARAM) || - paramSymbol.get().getModule().isEmpty() || - !validateModuleId(paramSymbol.get().getModule().get())) { - context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_FIRST_PARAMETER, - DiagnosticSeverity.ERROR, parameterNode.location())); - } - } else if (!paramSyntaxKind.equals(ERROR_TYPE_DESC)) { + TypeSymbol normalizedParamType = unwrapTypeReference(paramType.get()); + boolean isError = isSameType(normalizedParamType, semanticModel.types().ERROR); + boolean isFtpErrorSubtype = findFtpErrorTypeSymbol(semanticModel, "") + .map(this::unwrapTypeReference) + .map(normalizedParamType::subtypeOf) + .orElse(false); + if (!isError && !isFtpErrorSubtype) { context.reportDiagnostic(getDiagnostic(INVALID_ON_ERROR_FIRST_PARAMETER, DiagnosticSeverity.ERROR, parameterNode.location())); } } + private Optional findFtpErrorTypeSymbol(SemanticModel semanticModel, String version) { + Optional ftpErrorSymbol = semanticModel.types() + .getTypeByName(PACKAGE_ORG, PACKAGE_PREFIX, version, ERROR_PARAM); + if (ftpErrorSymbol.isEmpty()) { + return Optional.empty(); + } + Symbol symbol = ftpErrorSymbol.get(); + if (symbol instanceof TypeDefinitionSymbol typeDefinitionSymbol) { + return Optional.of(typeDefinitionSymbol.typeDescriptor()); + } + if (symbol instanceof TypeSymbol typeSymbol) { + return Optional.of(typeSymbol); + } + return Optional.empty(); + } + + private boolean isSameType(TypeSymbol left, TypeSymbol right) { + return left.subtypeOf(right) && right.subtypeOf(left); + } + + private TypeSymbol unwrapTypeReference(TypeSymbol typeSymbol) { + if (typeSymbol.typeKind() == TypeDescKind.TYPE_REFERENCE && + typeSymbol instanceof TypeReferenceTypeSymbol typeReferenceTypeSymbol) { + return typeReferenceTypeSymbol.typeDescriptor(); + } + return typeSymbol; + } + }