Skip to content
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

Add WebSocket Validation Feature #3924

Closed
wants to merge 1 commit into from

Conversation

krishpranav
Copy link

Issue Link πŸ”—

WebSocketRequest Should Support Validation #3853

Goals ⚽

This pull request introduces a validation feature for WebSocket communication. The feature ensures that all outgoing requests contain valid status codes and that the responses adhere to the expected format. This addition improves the robustness of our WebSocket communication by preventing invalid requests and ensuring consistent error handling.

Implementation Details 🚧

  • Validation for Status Codes: Added checks to ensure that status codes returned by the WebSocket server are valid and within the expected range.
  • Request Validation: Introduced validation to check the format and correctness of incoming WebSocket requests before processing them.
  • Error Handling: Implemented better error handling for invalid status codes or requests, providing clearer error messages to users and developers.
  • Extensibility: Designed the validation feature to be easily extensible in the future, allowing for custom validations if needed.

Testing Details πŸ”

webSocketRequest.validate { request, response, error in
    // Perform custom validation checks
    if let error = error {
        return .failure(error) // Fail if there is an error
    }
    
    if response?.statusCode != 200 {
        return .failure(AFError.responseValidationFailed(reason: .unacceptableStatusCode(code: response?.statusCode ?? 0)))
    }
    
    return .success // Return success if no issues found
}

@krishpranav
Copy link
Author

fix for WebSocketRequest Should Support Validation #3853

@jshier
Copy link
Contributor

jshier commented Dec 9, 2024

I'm sorry, but this doesn't seem to do any sort of validation. validate() is automatic, and unfortunately URLSessionWebSocketTask doesn't actually provide the hooks necessary for that automatic validation to actually occur. Adding manual validation like this doesn't really do anything, as users could just put that logic in the response handlers themselves.

@jshier jshier closed this Dec 9, 2024
@krishpranav
Copy link
Author

ok no problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants