Known Issues*

[CodingAleCR]

Known Issues in http_interceptor

This document outlines the remaining issues found in the codebase that should be addressed for improved robustness and reliability.

🔧 Issues by Priority

High Priority Issues

1. Potential Memory Leak: Timer Not Always Cancelled

Location: lib/http/intercepted_client.dart lines 340-348
Status: ⚠️ Needs Investigation

Issue: The timeout timer might not be cancelled in all code paths, potentially causing memory leaks.

timeoutTimer = Timer(requestTimeout!, () {
  // ... timeout logic ...
});

requestFuture.then((streamResponse) {
  timeoutTimer?.cancel(); // ✅ Cancelled here
  // ... code ...
}).catchError((error) {
  timeoutTimer?.cancel(); // ✅ Cancelled here
  // ... code ...
});

Risk: If the completer is completed through the timeout callback, the timer might not be cancelled in all scenarios.

Suggested Fix: Ensure timer cancellation in all completion paths and add proper cleanup.

---

2. Race Condition: Completer State Management

Location: lib/http/intercepted_client.dart lines 310-350
Status: ⚠️ Needs Investigation

Issue: Multiple code paths can complete the same completer, potentially causing race conditions.

if (!completer.isCompleted) {
  completer.complete(response);
}

Risk: Multiple threads or async operations could try to complete the same completer simultaneously.

Suggested Fix: Add proper synchronization or use atomic operations for completer state management.

---

Medium Priority Issues

3. Potential Null Reference: Request Body Handling

Location: lib/http/intercepted_client.dart lines 240-251
Status: 🔧 Needs Improvement

Issue: The body type checking doesn't handle all possible null cases properly.

if (body != null) {
  if (body is String) {
    request.body = body;
  } else if (body is List) {
    request.bodyBytes = body.cast<int>(); // ❌ Could fail if List is null
  } else if (body is Map) {
    request.bodyFields = body.cast<String, String>(); // ❌ Could fail if Map is null
  } else {
    throw ArgumentError('Invalid request body "$body".');
  }
}

Suggested Fix: Add null checks and better type validation.

---

4. Inconsistent Error Handling: StreamedResponse Type Check

Location: lib/http/intercepted_client.dart lines 220-227
Status: 🔧 Needs Improvement

Issue: The type check for StreamedResponse throws a generic ClientException instead of a more specific error.

if (interceptedResponse is StreamedResponse) {
  return interceptedResponse;
}

throw ClientException(
  'Expected `StreamedResponse`, got ${interceptedResponse.runtimeType}.',
);

Impact: This makes debugging harder as the error message doesn't provide enough context.

Suggested Fix: Create a more specific exception type or improve error messages.

---

5. Potential Issue: URL Parameter Encoding

Location: lib/utils/query_parameters.dart lines 42-60
Status: 🔧 Needs Investigation

Issue: The URL parameter encoding logic might not handle all edge cases properly.

parameters.forEach((key, value) {
  final encodedKey = Uri.encodeQueryComponent(key);
  
  if (value is List) {
    // ... list handling ...
  } else if (value is String) {
    url += "$encodedKey=${Uri.encodeQueryComponent(value)}&";
  } else {
    url += "$encodedKey=${Uri.encodeQueryComponent(value.toString())}&";
  }
});

Risk: If value.toString() returns null or throws an exception, this could cause issues.

Suggested Fix: Add proper null checks and exception handling.

---

Low Priority Issues

6. Potential Issue: Retry Policy State Management

Location: lib/http/intercepted_client.dart lines 360-380
Status: 🔧 Needs Improvement

Issue: The retry count is reset for each new request, but there's no validation that retry attempts don't exceed reasonable limits.

_retryCount = 0; // Reset retry count for each new request

Risk: A malicious retry policy could cause infinite loops or excessive retries.

Suggested Fix: Add maximum retry limits and validation.

---

7. Minor Issue: Inconsistent Exception Types

Location: lib/models/http_interceptor_exception.dart
Status: 🔧 Needs Improvement

Issue: The custom exception class uses dynamic for the message, which could lead to type safety issues.

class HttpInterceptorException implements Exception {
  final dynamic message; // ❌ Should be String?
  
  HttpInterceptorException([this.message]);
}

Suggested Fix: Change to String? for better type safety.

---

8. Potential Issue: Extension Type Safety

Location: lib/extensions/base_request.dart lines 15-70
Status: 🔧 Needs Improvement

Issue: The copyWith method uses dynamic for the body parameter, which could cause runtime errors.

BaseRequest copyWith({
  // ... other parameters ...
  dynamic body, // ❌ Should be more specific
  // ... other parameters ...
}) {
  // ... implementation ...
}

Suggested Fix: Use more specific types or add runtime type checking.

---

9. Minor Issue: Missing Validation in HTTP Methods

Location: lib/http/http_methods.dart lines 15-29
Status: 🔧 Needs Improvement

Issue: The fromString method doesn't handle case-insensitive parsing or trim whitespace.

static HttpMethod fromString(String method) {
  switch (method) { // ❌ Case-sensitive, no trimming
    case "HEAD":
      return HttpMethod.HEAD;
    // ... other cases ...
  }
  throw ArgumentError.value(method, "method", "Must be a valid HTTP Method.");
}

Suggested Fix: Add case-insensitive parsing and whitespace trimming.

---

🧪 Testing Issues

Test Coverage Gaps

10. Missing Edge Case Tests

Status: 📝 Needs Additional Tests

Areas needing more test coverage:

• Timer cancellation edge cases
• Completer race condition scenarios
• Null body handling
• Malformed URL parameter handling
• Retry policy edge cases
• Exception type validation

Suggested Action: Add comprehensive edge case tests for all identified issues.

---

📋 Action Items

Immediate Actions (High Priority)

1. 🔧 Investigate and fix timer cancellation issues
2. 🔧 Add proper synchronization for completer state management

Short-term Actions (Medium Priority)

3. 🔧 Improve request body handling with better null checks
4. 🔧 Create more specific exception types for better error handling
5. 🔧 Add comprehensive URL parameter encoding validation
6. 🔧 Add retry policy validation and limits

Long-term Actions (Low Priority)

7. 🔧 Improve type safety in exception classes
8. 🔧 Add case-insensitive HTTP method parsing
9. 🔧 Enhance extension type safety
10. 📝 Add comprehensive edge case tests

---

🎯 Success Criteria

For High Priority Issues

• All timers are properly cancelled in all code paths
• No race conditions in completer state management
• Comprehensive tests for edge cases

For Medium Priority Issues

• Robust null handling in request body processing
• Specific and informative error messages
• Validated URL parameter encoding

For Low Priority Issues

• Improved type safety throughout the codebase
• Better HTTP method parsing
• Enhanced test coverage

---

📊 Issue Summary

Priority	Count	Status
High	2	1 Fixed, 1 Pending
Medium	3	All Pending
Low	4	All Pending
Testing	1	Pending

Total Issues: 10
Fixed: 1
Pending: 9

---

🔍 Monitoring

Areas to Monitor

1. Memory usage patterns in long-running applications
2. Exception frequency and types in production
3. Performance impact of retry policies
4. URL encoding edge cases in real-world usage

Metrics to Track

• Timer cancellation success rate
• Completer completion patterns
• Exception type distribution
• Retry attempt frequency

---

📚 References

• Dart Language Tour - Error Handling
• Dart Best Practices - Async Programming
• HTTP Package Documentation

---

Last Updated: [Current Date]
Status: Active Monitoring Required

[techouse]

@CodingAleCR, thanks for highlighting these. Here are some of my comments on the issues listed above:

• 3. is not an issue. Unlike suggested

  if (body != null) { // ✅ already checking it it's not null
    if (body is String) {
      request.body = body;
    } else if (body is List) {
      request.bodyBytes = body.cast<int>(); // ✅ it's checking for a `List` not a `List?`, hence it can not be null
    } else if (body is Map) {
      request.bodyFields = body.cast<String, String>(); // ✅ it's checking for a `Map` not `Map?`, hence it can not be null
    } else {
      throw ArgumentError('Invalid request body "$body".');
    }
  }

• 4. this should be categorized as a low priority issue imho
• 5. got fixed with feat: use qs_dart in buildUrlString for enhanced query string encoding #158

---

Another issue, not listed above, is that StreamedRequestCopyWith.copyWith doesn’t work. You can try it yourself.

import 'dart:convert';

import 'package:http_interceptor/http_interceptor.dart';
import 'package:test/test.dart';

void main() {
  group('StreamedRequest.copyWith', () {
    late StreamedRequest request;
    final testUrl = Uri.parse('https://example.com');
    final testHeaders = {'Content-Type': 'application/json'};

    setUp(() {
      request = StreamedRequest('POST', testUrl)
        ..headers.addAll(testHeaders);

      // Add test data to the request
      final testData = utf8.encode('test data');
      request.sink.add(testData);
      request.sink.close();
    });

    test('creates a copy with the same properties when no parameters are provided',
        () async {
      // Act
      final copy = request.copyWith();

      // Assert
      expect(copy.method, equals(request.method));
      expect(copy.url, equals(request.url));
      expect(copy.headers, equals(request.headers));

      // Collect the data from both streams to compare
      final originalData = await request.finalize().toBytes();
      final copyData = await copy.finalize().toBytes();
      expect(copyData, equals(originalData));

      expect(copy.followRedirects, equals(true)); // Default value
      expect(copy.maxRedirects, equals(5)); // Default value
      expect(copy.persistentConnection, equals(true)); // Default value
    });
  }
}

This will throw

package:http/src/base_request.dart 163:5                         BaseRequest._checkFinalized
package:http/src/base_request.dart 55:5                          BaseRequest.persistentConnection=
package:http_interceptor/extensions/streamed_request.dart 32:10  StreamedRequestCopyWith.copyWith
test/extensions/streamed_request_test.dart 25:28                 main.<fn>.<fn>

Bad state: Can't modify a finalized Request.

I’ve addressed this issue in #169, but it modifies the method signature from StreamedRequest copyWith to Future<StreamedRequest> copyWith. As a result, all subsequent copyWith calls will need to be awaited.