Skip to content

Commit 5096a9e

Browse files
kirkmorrowkirkmorrow
committed
PR feedback fixes
1 parent bb98ddd commit 5096a9e

File tree

5 files changed

+53
-55
lines changed

5 files changed

+53
-55
lines changed

‎packages/dart/lib/src/network/parse_dio_client.dart‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'dart:convert';
2+
13
import 'package:dio/dio.dart' as dio;
24
import 'package:parse_server_sdk/parse_server_sdk.dart';
35

@@ -90,8 +92,7 @@ class ParseDioClient extends ParseClient {
9092
);
9193
} else {
9294
return ParseNetworkByteResponse(
93-
data:
94-
"{\"code\":${ParseError.otherCause},\"error\":\"${error.error.toString()}\"}",
95+
data: _buildErrorJson(error.error.toString()),
9596
statusCode: ParseError.otherCause,
9697
);
9798
}
@@ -199,11 +200,23 @@ class ParseDioClient extends ParseClient {
199200

200201
ParseNetworkResponse _getOtherCaseErrorForParseNetworkResponse(String error) {
201202
return ParseNetworkResponse(
202-
data: "{\"code\":${ParseError.otherCause},\"error\":\"$error\"}",
203+
data: _buildErrorJson(error),
203204
statusCode: ParseError.otherCause,
204205
);
205206
}
206207

208+
/// Builds a properly escaped JSON error payload.
209+
///
210+
/// This helper ensures error messages are safely escaped to prevent
211+
/// malformed JSON when the message contains quotes or special characters.
212+
String _buildErrorJson(String errorMessage) {
213+
final Map<String, dynamic> errorPayload = <String, dynamic>{
214+
'code': ParseError.otherCause,
215+
'error': errorMessage,
216+
};
217+
return jsonEncode(errorPayload);
218+
}
219+
207220
@override
208221
Future<ParseNetworkResponse> delete(
209222
String path, {
@@ -231,8 +244,7 @@ class ParseDioClient extends ParseClient {
231244
);
232245
}
233246

234-
String get _fallbackErrorData =>
235-
'{"code":${ParseError.otherCause},"error":"NetworkError"}';
247+
String get _fallbackErrorData => _buildErrorJson('NetworkError');
236248
}
237249

238250
/// Creates a custom version of HTTP Client that has Parse Data Preset

‎packages/dart/lib/src/network/parse_http_client.dart‎

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,7 @@ class ParseHTTPClient extends ParseClient {
6060
);
6161
} catch (e) {
6262
return ParseNetworkResponse(
63-
data:
64-
'{"code":${ParseError.otherCause},"error":"NetworkError","exception":"${e.toString()}"}',
63+
data: _buildErrorJson(e.toString()),
6564
statusCode: ParseError.otherCause,
6665
);
6766
}
@@ -89,8 +88,7 @@ class ParseHTTPClient extends ParseClient {
8988
);
9089
} catch (e) {
9190
return ParseNetworkByteResponse(
92-
data:
93-
'{"code":${ParseError.otherCause},"error":"NetworkError","exception":"${e.toString()}"}',
91+
data: _buildErrorJson(e.toString()),
9492
statusCode: ParseError.otherCause,
9593
);
9694
}
@@ -118,8 +116,7 @@ class ParseHTTPClient extends ParseClient {
118116
);
119117
} catch (e) {
120118
return ParseNetworkResponse(
121-
data:
122-
'{"code":${ParseError.otherCause},"error":"NetworkError","exception":"${e.toString()}"}',
119+
data: _buildErrorJson(e.toString()),
123120
statusCode: ParseError.otherCause,
124121
);
125122
}
@@ -147,8 +144,7 @@ class ParseHTTPClient extends ParseClient {
147144
);
148145
} catch (e) {
149146
return ParseNetworkResponse(
150-
data:
151-
'{"code":${ParseError.otherCause},"error":"NetworkError","exception":"${e.toString()}"}',
147+
data: _buildErrorJson(e.toString()),
152148
statusCode: ParseError.otherCause,
153149
);
154150
}
@@ -183,8 +179,7 @@ class ParseHTTPClient extends ParseClient {
183179
);
184180
} catch (e) {
185181
return ParseNetworkResponse(
186-
data:
187-
'{"code":${ParseError.otherCause},"error":"NetworkError","exception":"${e.toString()}"}',
182+
data: _buildErrorJson(e.toString()),
188183
statusCode: ParseError.otherCause,
189184
);
190185
}
@@ -210,14 +205,26 @@ class ParseHTTPClient extends ParseClient {
210205
);
211206
} catch (e) {
212207
return ParseNetworkResponse(
213-
data:
214-
'{"code":${ParseError.otherCause},"error":"NetworkError","exception":"${e.toString()}"}',
208+
data: _buildErrorJson(e.toString()),
215209
statusCode: ParseError.otherCause,
216210
);
217211
}
218212
},
219213
);
220214
}
215+
216+
/// Builds a properly escaped JSON error payload.
217+
///
218+
/// This helper ensures error messages are safely escaped to prevent
219+
/// malformed JSON when the message contains quotes or special characters.
220+
String _buildErrorJson(String exceptionMessage) {
221+
final Map<String, dynamic> errorPayload = <String, dynamic>{
222+
'code': ParseError.otherCause,
223+
'error': 'NetworkError',
224+
'exception': exceptionMessage,
225+
};
226+
return jsonEncode(errorPayload);
227+
}
221228
}
222229

223230
/// Creates a custom version of HTTP Client that has Parse Data Preset

‎packages/dart/lib/src/network/parse_network_retry.dart‎

Lines changed: 13 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,8 @@ Future<T> executeWithRetry<T extends ParseNetworkResponse>({
6868
const int maxRetries = 100;
6969
if (retryIntervals.length > maxRetries) {
7070
throw ArgumentError(
71-
'restRetryIntervals cannot exceed $maxRetries retries. '
71+
'restRetryIntervals cannot exceed $maxRetries elements '
72+
'(which allows up to ${maxRetries + 1} total attempts). '
7273
'Current length: ${retryIntervals.length}',
7374
);
7475
}
@@ -136,8 +137,11 @@ Future<T> executeWithRetry<T extends ParseNetworkResponse>({
136137
}
137138
}
138139

139-
// Should never reach here, but return last response as fallback
140-
return lastResponse!;
140+
// This should never be reached due to the loop logic above
141+
throw StateError(
142+
'Retry loop completed without returning or rethrowing. '
143+
'This indicates a logic error.',
144+
);
141145
}
142146

143147
/// Determines if a network response should be retried.
@@ -148,32 +152,16 @@ Future<T> executeWithRetry<T extends ParseNetworkResponse>({
148152
/// Retry Triggers:
149153
///
150154
/// - Status code `-1` (network/parsing errors)
151-
/// - Response body starts with HTML tags (proxy/load balancer errors)
155+
/// - HTML responses from proxies/load balancers (502, 503, 504 errors)
156+
/// - Socket exceptions, timeouts, DNS failures
157+
/// - JSON parse errors from malformed responses
152158
///
153159
/// No Retry:
154160
///
155161
/// - Status code 200 or 201 (success)
156162
/// - Valid Parse Server error codes (e.g., 100-series errors)
163+
/// - These are application-level errors that won't resolve with retries
157164
bool _shouldRetryResponse(ParseNetworkResponse response) {
158-
// Retry on status code -1 (network/parse errors)
159-
if (response.statusCode == ParseError.otherCause) {
160-
// Additional check: is it HTML instead of JSON?
161-
final String trimmedData = response.data.trimLeft().toLowerCase();
162-
163-
// Check for common HTML patterns that indicate proxy/load balancer errors
164-
// More robust than just checking for '<' which could be in valid JSON strings
165-
if (trimmedData.startsWith('<!doctype') ||
166-
trimmedData.startsWith('<html') ||
167-
trimmedData.startsWith('<head') ||
168-
trimmedData.startsWith('<body')) {
169-
// HTML response indicates proxy/load balancer error
170-
return true;
171-
}
172-
173-
// Other -1 errors (network issues, parse failures)
174-
return true;
175-
}
176-
177-
// Don't retry successful responses or valid Parse API errors
178-
return false;
165+
// Retry all -1 status codes (network/parse errors, including HTML from proxies)
166+
return response.statusCode == ParseError.otherCause;
179167
}

‎packages/dart/test/src/network/parse_client_retry_integration_test.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ void main() {
149149
final fetchedObject = await object.fetch();
150150

151151
expect(callCount, 1); // No retry via mock
152-
expect(fetchedObject.objectId, 'abc123');
152+
expect(fetchedObject.objectId, 'abc123'); // Original objectId preserved
153153
},
154154
);
155155

‎packages/dart/test/src/network/parse_network_retry_test.dart‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ void main() {
149149
final oldIntervals = ParseCoreData().restRetryIntervals;
150150
ParseCoreData().restRetryIntervals = [0, 10];
151151

152-
expect(
153-
() async => await executeWithRetry(
152+
await expectLater(
153+
executeWithRetry(
154154
operation: () async {
155155
callCount++;
156156
throw Exception('Network timeout');
@@ -166,7 +166,6 @@ void main() {
166166
);
167167

168168
// Should retry on exceptions: initial + 2 retries = 3 times
169-
await Future.delayed(Duration(milliseconds: 50)); // Wait for retries
170169
expect(callCount, 3);
171170

172171
ParseCoreData().restRetryIntervals = oldIntervals;
@@ -495,7 +494,7 @@ void main() {
495494
isA<ArgumentError>().having(
496495
(e) => e.message,
497496
'message',
498-
contains('cannot exceed 100 retries'),
497+
contains('cannot exceed 100 elements'),
499498
),
500499
),
501500
);
@@ -518,13 +517,5 @@ void main() {
518517
expect(result.data, contains('"code"'));
519518
expect(result.data, contains('"error"'));
520519
});
521-
522-
test('should work with both HTTP client implementations', () async {
523-
// This test documents that both ParseDioClient and ParseHTTPClient
524-
// should use the same error format
525-
final errorFormat = '{"code":-1,"error":"NetworkError"}';
526-
expect(errorFormat, contains('"code":'));
527-
expect(errorFormat, contains('"error":'));
528-
});
529520
});
530521
}

0 commit comments

Comments
 (0)