Skip to content

Commit 8dd576f

Browse files
authored
Merge pull request #1530 from niveathika/error-structure
Add distinct error types
2 parents 2ab4ca2 + 49223c7 commit 8dd576f

21 files changed

+1012
-87
lines changed

ballerina/Dependencies.toml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ distribution-version = "2201.12.0"
1010
[[package]]
1111
org = "ballerina"
1212
name = "crypto"
13-
version = "2.10.0"
13+
version = "2.10.1"
1414
dependencies = [
1515
{org = "ballerina", name = "jballerina.java"},
1616
{org = "ballerina", name = "time"}
@@ -46,7 +46,7 @@ modules = [
4646
[[package]]
4747
org = "ballerina"
4848
name = "data.xmldata"
49-
version = "1.5.2"
49+
version = "1.5.3"
5050
dependencies = [
5151
{org = "ballerina", name = "jballerina.java"},
5252
{org = "ballerina", name = "lang.object"}
@@ -215,7 +215,7 @@ modules = [
215215
[[package]]
216216
org = "ballerina"
217217
name = "observe"
218-
version = "1.5.0"
218+
version = "1.5.1"
219219
dependencies = [
220220
{org = "ballerina", name = "jballerina.java"}
221221
]
@@ -233,7 +233,7 @@ dependencies = [
233233
[[package]]
234234
org = "ballerina"
235235
name = "task"
236-
version = "2.11.0"
236+
version = "2.11.1"
237237
dependencies = [
238238
{org = "ballerina", name = "jballerina.java"},
239239
{org = "ballerina", name = "time"},

ballerina/error.bal

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,23 @@
1616

1717
# Defines the common error type for the module.
1818
public type Error distinct error;
19+
20+
# Represents an error that occurs when connecting to the FTP/SFTP server.
21+
# This includes network failures, host unreachable, connection refused, etc.
22+
public type ConnectionError distinct Error;
23+
24+
# Represents an error that occurs when a requested file or directory is not found.
25+
public type FileNotFoundError distinct Error;
26+
27+
# Represents an error that occurs when attempting to create a file or directory that already exists.
28+
public type FileAlreadyExistsError distinct Error;
29+
30+
# Represents an error that occurs when FTP/SFTP configuration is invalid.
31+
# This includes invalid port numbers, invalid regex patterns, invalid timeout values, etc.
32+
public type InvalidConfigError distinct Error;
33+
34+
# Represents an error that occurs when the FTP/SFTP service is temporarily unavailable.
35+
# This is a transient error indicating the operation may succeed on retry.
36+
# Common causes include: server overload (421), connection issues (425, 426),
37+
# temporary file locks (450), or server-side processing errors (451).
38+
public type ServiceUnavailableError distinct Error;

ballerina/tests/advanced_file_selection_test.bal

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ function testFileAgeFilterRespectsMaxAge() returns error? {
6565
}
6666
},
6767
path: "/home/in/advanced/age",
68-
pollingInterval: 5,
68+
pollingInterval: 2,
6969
fileAgeFilter: {
7070
maxAge: 60
7171
},

ballerina/tests/client_endpoint_negative_test.bal

Lines changed: 130 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@ public function testConnectionWithNonExistingServer() returns error? {
2828
auth: {credentials: {username: "wso2", password: "wso2123"}}
2929
};
3030
Client|Error nonExistingServerClientEp = new (nonExistingServerConfig);
31-
if nonExistingServerClientEp is Error {
31+
test:assertTrue(nonExistingServerClientEp is ConnectionError,
32+
msg = "Expected ConnectionError when connecting to non-existing server");
33+
if nonExistingServerClientEp is ConnectionError {
3234
test:assertTrue(nonExistingServerClientEp.message().startsWith("Error while connecting to the FTP server with URL: "),
3335
msg = "Unexpected error when tried to connect to a non existing server. " + nonExistingServerClientEp.message());
34-
} else {
35-
test:assertFail(msg = "Found a non-error response when tried to connect to a non existing server.");
3636
}
3737
}
3838

@@ -60,11 +60,11 @@ public function testConnectionWithInvalidConfiguration() returns error? {
6060
}
6161
public function testReadNonExistingFile() returns error? {
6262
stream<byte[] & readonly, io:Error?>|Error str = (<Client>clientEp)->get("/home/in/nonexisting.txt");
63-
if str is Error {
63+
test:assertTrue(str is FileNotFoundError,
64+
msg = "Expected FileNotFoundError when reading non-existing file");
65+
if str is FileNotFoundError {
6466
test:assertEquals(str.message(), "Failed to read file: ftp://wso2:***@127.0.0.1:21212/home/in/nonexisting.txt not found",
6567
msg = "Unexpected error during the `get` operation of an non-existing file.");
66-
} else {
67-
test:assertFail(msg = "Found a non-error response while accessing a non-existing file path");
6868
}
6969
}
7070

@@ -101,11 +101,11 @@ public function testPutFileContentAtInvalidFileLocation() returns error? {
101101
}
102102
public function testIsDirectoryWithNonExistingDirectory() {
103103
boolean|Error receivedError = (<Client>clientEp)->isDirectory("/home/in/nonexisting");
104-
if receivedError is Error {
105-
test:assertEquals(receivedError.message(), "/home/in/nonexisting does not exists to check if it is a directory.",
104+
test:assertTrue(receivedError is FileNotFoundError,
105+
msg = "Expected FileNotFoundError when checking isDirectory on non-existing path");
106+
if receivedError is FileNotFoundError {
107+
test:assertEquals(receivedError.message(), "/home/in/nonexisting does not exist to check if it is a directory.",
106108
msg = "Unexpected error during the `isDirectory` operation of an non-existing directory. " + receivedError.message());
107-
} else {
108-
test:assertFail(msg = "Found a non-error response while accessing a non-existing directory path.");
109109
}
110110
}
111111

@@ -130,11 +130,11 @@ public function testRenameNonExistingDirectory() {
130130
string newName = "/home/in/differentDirectory";
131131
Error? receivedError = (<Client>clientEp)->rename(existingName, newName);
132132

133-
if receivedError is Error {
133+
test:assertTrue(receivedError is FileNotFoundError,
134+
msg = "Expected FileNotFoundError when renaming non-existing file");
135+
if receivedError is FileNotFoundError {
134136
test:assertTrue(receivedError.message().startsWith("Failed to rename file: "),
135137
msg = "Unexpected error during the `rename` operation of an non-existing file. " + receivedError.message());
136-
} else {
137-
test:assertFail(msg = "Found a non-error response while accessing a non existing directory path.");
138138
}
139139
}
140140

@@ -169,11 +169,11 @@ public function testListFilesFromNonExistingDirectory() {
169169
}
170170
public function testDeleteFileAtNonExistingLocation() returns error? {
171171
Error? receivedError = (<Client>clientEp)->delete("/nonExistingFile");
172-
if receivedError is Error {
172+
test:assertTrue(receivedError is FileNotFoundError,
173+
msg = "Expected FileNotFoundError when deleting non-existing file");
174+
if receivedError is FileNotFoundError {
173175
test:assertTrue(receivedError.message().startsWith("Failed to delete file: "),
174176
msg = "Unexpected error during the `delete` operation of an non-existing file. " + receivedError.message());
175-
} else {
176-
test:assertFail(msg = "Found a non-error response while accessing a non-existing file path.");
177177
}
178178
}
179179

@@ -182,11 +182,11 @@ public function testDeleteFileAtNonExistingLocation() returns error? {
182182
}
183183
public function testRemoveDirectoryWithWrongUrl() {
184184
Error? receivedError = (<Client>clientEp)->rmdir("/nonExistingDirectory");
185-
if receivedError is Error {
185+
test:assertTrue(receivedError is FileNotFoundError,
186+
msg = "Expected FileNotFoundError when removing non-existing directory");
187+
if receivedError is FileNotFoundError {
186188
test:assertTrue(receivedError.message().startsWith("Failed to delete directory: "),
187189
msg = "Unexpected error during the `rmdir` operation of a non-existing directory. " + receivedError.message());
188-
} else {
189-
test:assertFail(msg = "Found a non-error response while accessing a non-existing directory path.");
190190
}
191191
}
192192

@@ -227,14 +227,14 @@ public function testConnectionWithNonExistingServerDetailedError() returns error
227227
auth: {credentials: {username: "wso2", password: "wso2123"}}
228228
};
229229
Client|Error nonExistingServerClientEp = new (nonExistingServerConfig);
230-
if nonExistingServerClientEp is Error {
230+
test:assertTrue(nonExistingServerClientEp is ConnectionError,
231+
msg = "Expected ConnectionError when connecting to non-existing server");
232+
if nonExistingServerClientEp is ConnectionError {
231233
test:assertTrue(nonExistingServerClientEp.message().startsWith("Error while connecting to the FTP server with URL: "),
232234
msg = "Unexpected error when tried to connect to a non existing server. " + nonExistingServerClientEp.message());
233235
// Verify that the error message contains additional details from the root cause
234236
test:assertTrue(nonExistingServerClientEp.message().length() > "Error while connecting to the FTP server with URL: ftp://wso2:***@127.0.0.1:21299".length(),
235237
msg = "Error message should contain detailed root cause information");
236-
} else {
237-
test:assertFail(msg = "Found a non-error response when tried to connect to a non existing server.");
238238
}
239239
}
240240

@@ -249,14 +249,14 @@ public function testConnectionWithInvalidHostDetailedError() returns error? {
249249
auth: {credentials: {username: "wso2", password: "wso2123"}}
250250
};
251251
Client|Error invalidHostClientEp = new (invalidHostConfig);
252-
if invalidHostClientEp is Error {
252+
test:assertTrue(invalidHostClientEp is ConnectionError,
253+
msg = "Expected ConnectionError when connecting to invalid host");
254+
if invalidHostClientEp is ConnectionError {
253255
test:assertTrue(invalidHostClientEp.message().startsWith("Error while connecting to the FTP server with URL: "),
254256
msg = "Unexpected error when tried to connect to an invalid host. " + invalidHostClientEp.message());
255257
// Verify that the error message contains additional details from the root cause
256258
test:assertTrue(invalidHostClientEp.message().length() > "Error while connecting to the FTP server with URL: ".length(),
257259
msg = "Error message should contain detailed root cause information");
258-
} else {
259-
test:assertFail(msg = "Found a non-error response when tried to connect to an invalid host.");
260260
}
261261
}
262262

@@ -266,12 +266,13 @@ public function testConnectionWithInvalidHostDetailedError() returns error? {
266266
public function testMoveNonExistingFile() returns error? {
267267
string nonExistingPath = "/home/in/nonexistent_file_for_move.txt";
268268
string destinationPath = "/home/in/moved_nonexistent.txt";
269-
// Note: VFS may not throw error for non-existing source in move/rename operations
270-
// This test verifies that the operation completes without crashing
271269
Error? result = (<Client>clientEp)->move(nonExistingPath, destinationPath);
272-
// If the VFS implementation doesn't throw an error, that's also acceptable
273-
// The important thing is the client handles it gracefully
274-
test:assertTrue(true, msg = "Move non-existing file handled");
270+
test:assertTrue(result is FileNotFoundError,
271+
msg = "Expected FileNotFoundError when moving non-existing file");
272+
if result is FileNotFoundError {
273+
test:assertTrue(result.message().includes("not found"),
274+
msg = "Error message should indicate file not found");
275+
}
275276
}
276277

277278
@test:Config {
@@ -280,12 +281,13 @@ public function testMoveNonExistingFile() returns error? {
280281
public function testCopyNonExistingFile() returns error? {
281282
string nonExistingPath = "/home/in/nonexistent_file_for_copy.txt";
282283
string destinationPath = "/home/in/copied_nonexistent.txt";
283-
// Note: VFS may not throw error for non-existing source in copy operations
284-
// This test verifies that the operation completes without crashing
285284
Error? result = (<Client>clientEp)->copy(nonExistingPath, destinationPath);
286-
// If the VFS implementation doesn't throw an error, that's also acceptable
287-
// The important thing is the client handles it gracefully
288-
test:assertTrue(true, msg = "Copy non-existing file handled");
285+
test:assertTrue(result is FileNotFoundError,
286+
msg = "Expected FileNotFoundError when copying non-existing file");
287+
if result is FileNotFoundError {
288+
test:assertTrue(result.message().includes("not found"),
289+
msg = "Error message should indicate file not found");
290+
}
289291
}
290292

291293
@test:Config {
@@ -294,27 +296,28 @@ public function testCopyNonExistingFile() returns error? {
294296
public function testCopyToExistingFile() returns error? {
295297
string sourcePath = "/home/in/test_copy_source.txt";
296298
string destinationPath = "/home/in/test_copy_dest.txt";
297-
299+
298300
// Clean up any existing files first
299301
Error? cleanupResult1 = (<Client>clientEp)->delete(sourcePath);
300302
Error? cleanupResult2 = (<Client>clientEp)->delete(destinationPath);
301-
303+
302304
// Create source file
303305
stream<io:Block, io:Error?> sourceStream = check io:fileReadBlocksAsStream(putFilePath, 5);
304306
check (<Client>clientEp)->put(sourcePath, sourceStream);
305-
307+
306308
// Create destination file
307309
stream<io:Block, io:Error?> destStream = check io:fileReadBlocksAsStream(putFilePath, 5);
308310
check (<Client>clientEp)->put(destinationPath, destStream);
309-
310-
// Copy to existing file should fail (VFS doesn't overwrite by default)
311+
312+
// Copy to existing file should fail with FileAlreadyExistsError
311313
Error? result = (<Client>clientEp)->copy(sourcePath, destinationPath);
312-
test:assertTrue(result is Error, msg = "Copy to existing file should fail");
313-
if result is Error {
314-
test:assertTrue(result.message().includes("already exists"),
314+
test:assertTrue(result is FileAlreadyExistsError,
315+
msg = "Expected FileAlreadyExistsError when copying to existing file");
316+
if result is FileAlreadyExistsError {
317+
test:assertTrue(result.message().includes("already exists"),
315318
msg = "Error message should indicate file already exists");
316319
}
317-
320+
318321
// Cleanup
319322
Error? cleanup1 = (<Client>clientEp)->delete(sourcePath);
320323
Error? cleanup2 = (<Client>clientEp)->delete(destinationPath);
@@ -329,7 +332,89 @@ public function testExistsNonExistingFile() returns error? {
329332
if result is boolean {
330333
test:assertFalse(result, msg = "Non-existing file should return false");
331334
} else {
332-
test:assertFail(msg = "Expected boolean result for exists on non-existing file, got error: "
335+
test:assertFail(msg = "Expected boolean result for exists on non-existing file, got error: "
333336
+ result.message());
334337
}
335338
}
339+
340+
// Invalid Configuration Tests
341+
342+
@test:Config {}
343+
public function testListenerWithInvalidFileNamePattern() returns error? {
344+
ListenerConfiguration invalidPatternConfig = {
345+
protocol: FTP,
346+
host: "127.0.0.1",
347+
port: 21212,
348+
auth: {credentials: {username: "wso2", password: "wso2123"}},
349+
path: "/home/in",
350+
pollingInterval: 2,
351+
fileNamePattern: "[invalid(regex" // Invalid regex pattern (unclosed bracket)
352+
};
353+
Listener|Error listenerResult = new (invalidPatternConfig);
354+
test:assertTrue(listenerResult is InvalidConfigError,
355+
msg = "Expected InvalidConfigError when creating listener with invalid fileNamePattern");
356+
if listenerResult is InvalidConfigError {
357+
test:assertTrue(listenerResult.message().includes("Invalid regex pattern"),
358+
msg = "Error message should indicate invalid regex pattern. Got: " + listenerResult.message());
359+
test:assertTrue(listenerResult.message().includes("fileNamePattern"),
360+
msg = "Error message should mention fileNamePattern field");
361+
}
362+
}
363+
364+
@test:Config {}
365+
public function testListenerWithInvalidDependencyTargetPattern() returns error? {
366+
ListenerConfiguration invalidDependencyConfig = {
367+
protocol: FTP,
368+
host: "127.0.0.1",
369+
port: 21212,
370+
auth: {credentials: {username: "wso2", password: "wso2123"}},
371+
path: "/home/in",
372+
pollingInterval: 2,
373+
fileDependencyConditions: [
374+
{
375+
targetPattern: "*.txt[invalid", // Invalid regex pattern
376+
requiredFiles: ["done.txt"],
377+
matchingMode: ALL,
378+
requiredFileCount: 1
379+
}
380+
]
381+
};
382+
Listener|Error listenerResult = new (invalidDependencyConfig);
383+
test:assertTrue(listenerResult is InvalidConfigError,
384+
msg = "Expected InvalidConfigError for invalid dependency targetPattern");
385+
if listenerResult is InvalidConfigError {
386+
test:assertTrue(listenerResult.message().includes("Invalid regex pattern"),
387+
msg = "Error message should indicate invalid regex pattern. Got: " + listenerResult.message());
388+
test:assertTrue(listenerResult.message().includes("targetPattern"),
389+
msg = "Error message should mention targetPattern field");
390+
}
391+
}
392+
393+
@test:Config {}
394+
public function testListenerWithInvalidDependencyRequiredFilePattern() returns error? {
395+
ListenerConfiguration invalidRequiredFileConfig = {
396+
protocol: FTP,
397+
host: "127.0.0.1",
398+
port: 21212,
399+
auth: {credentials: {username: "wso2", password: "wso2123"}},
400+
path: "/home/in",
401+
pollingInterval: 2,
402+
fileDependencyConditions: [
403+
{
404+
targetPattern: ".*\\.txt",
405+
requiredFiles: ["done[unclosed"], // Invalid regex pattern
406+
matchingMode: ALL,
407+
requiredFileCount: 1
408+
}
409+
]
410+
};
411+
Listener|Error listenerResult = new (invalidRequiredFileConfig);
412+
test:assertTrue(listenerResult is InvalidConfigError,
413+
msg = "Expected InvalidConfigError for invalid requiredFiles pattern");
414+
if listenerResult is InvalidConfigError {
415+
test:assertTrue(listenerResult.message().includes("Invalid regex pattern"),
416+
msg = "Error message should indicate invalid regex pattern. Got: " + listenerResult.message());
417+
test:assertTrue(listenerResult.message().includes("requiredFiles"),
418+
msg = "Error message should mention requiredFiles field");
419+
}
420+
}

ballerina/tests/client_endpoint_test.bal

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ public function testRenameDirectory() {
938938
boolean|Error response2 = (<Client>clientEp)->isDirectory(existingName);
939939
log:printInfo("Executed `isDirectory` operation on original directory after renaming a directory");
940940
if response2 is Error {
941-
test:assertEquals(response2.message(), "/home/in/out does not exists to check if it is a directory.",
941+
test:assertEquals(response2.message(), "/home/in/out does not exist to check if it is a directory.",
942942
msg = "Incorrect error message for non-existing file/directory at `isDirectory` operation");
943943
} else {
944944
test:assertFail("Error not created while invoking `isDirectory` operation after `rename` operation");
@@ -1247,7 +1247,7 @@ function testGenericRmdir(string path) returns error? {
12471247
i += 1;
12481248
}
12491249
if response2 is Error {
1250-
test:assertEquals(response2.message(), path + " does not exists to check if it is a directory.",
1250+
test:assertEquals(response2.message(), path + " does not exist to check if it is a directory.",
12511251
msg = "Incorrect error message for non-existing file/directory at `isDirectory` operation after `rmdir` operation");
12521252
} else {
12531253
// test:assertFail(msg = "Error not created while invoking `isDirectory` operation after `rmdir` operation on " + path );

0 commit comments

Comments
 (0)