-
Notifications
You must be signed in to change notification settings - Fork 174
fix(taskserver): resolve bugs and improve CCSync v3 integration #618
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,10 +1,11 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import 'dart:convert'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import 'package:http/http.dart' as http; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import 'package:flutter/material.dart'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import 'package:flutter/foundation.dart'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import 'package:path/path.dart'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Future<void> completeTask(String email, String taskUuid) async { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Future<void> completeTask(String email, String taskUuid, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| {http.Client? client}) async { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final httpClient = client ?? http.Client(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+6
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Resource leak: internally created When no client is injected, a new 🛡️ Proposed fix Future<void> completeTask(String email, String taskUuid,
{http.Client? client}) async {
- final httpClient = client ?? http.Client();
+ final httpClient = client;
+ final shouldCloseClient = httpClient == null;
+ final effectiveClient = httpClient ?? http.Client();
var c = await CredentialsStorage.getClientId();
var e = await CredentialsStorage.getEncryptionSecret();
var baseUrl = await CredentialsStorage.getApiUrl();
// ... rest of the code ...
try {
- final response = await httpClient.post(
+ final response = await effectiveClient.post(
// ...
);
// ...
} catch (e, s) {
debugPrint('Error completing task: $e\n$s');
rethrow;
+ } finally {
+ if (shouldCloseClient) {
+ effectiveClient.close();
+ }
}
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var c = await CredentialsStorage.getClientId(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var e = await CredentialsStorage.getEncryptionSecret(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| var baseUrl = await CredentialsStorage.getApiUrl(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -17,7 +18,7 @@ Future<void> completeTask(String email, String taskUuid) async { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final response = await http.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| final response = await httpClient.post( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Content-Type': 'application/json', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -29,13 +30,10 @@ Future<void> completeTask(String email, String taskUuid) async { | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| debugPrint('Task completed successfully on server'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| debugPrint('Failed to complete task: ${response.statusCode}'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ScaffoldMessenger.of(context as BuildContext).showSnackBar(const SnackBar( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| content: Text( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Failed to complete task!", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| style: TextStyle(color: Colors.red), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw Exception('Failed to complete task: ${response.statusCode}'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| debugPrint('Error completing task: $e'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (e, s) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| debugPrint('Error completing task: $e\n$s'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rethrow; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1,3 @@ | ||
| String origin = 'http://localhost:8080'; | ||
| import 'package:taskwarrior/app/utils/taskchampion/credentials_storage.dart'; | ||
|
|
||
| Future<String> getOrigin() async => await CredentialsStorage.getApiUrl() ?? ''; | ||
|
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Search for usages of getOrigin() across the codebase
# Search for imports of origin.dart
echo "=== Imports of origin.dart ==="
rg -l "import.*origin\.dart" --type dart
# Search for getOrigin function calls
echo -e "\n=== Usages of getOrigin() ==="
rg -n "getOrigin\(" --type dartRepository: CCExtractor/taskwarrior-flutter Length of output: 245 Remove unused The 🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| list(APPEND FLUTTER_PLUGIN_LIST | ||
| file_selector_linux | ||
| flutter_timezone | ||
| gtk | ||
| url_launcher_linux | ||
| ) | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent error handling:
deleteTaskis not awaited or wrapped in try-catch.completeTaskis now properly awaited with error handling, butdeleteTaskon line 86 is called withoutawaitand has no try-catch. This inconsistency could lead to unhandled errors ifdeleteTaskthrows, potentially causing silent failures or unhandled promise rejections.🛡️ Proposed fix for consistency
if (localTask.status == 'completed') { try { await completeTask('email', localTask.uuid!); } catch (e) { debugPrint('Failed to complete task on server: $e'); } } else if (localTask.status == 'deleted') { - deleteTask('email', localTask.uuid!); + try { + await deleteTask('email', localTask.uuid!); + } catch (e) { + debugPrint('Failed to delete task on server: $e'); + } }📝 Committable suggestion
🤖 Prompt for AI Agents