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

Fix server response closing #2

Merged
merged 2 commits into from
Jan 19, 2025
Merged

Conversation

cedricve
Copy link
Member

@cedricve cedricve commented Jan 19, 2025

Description

Pull Request Description

Title: Fix server response closing

Motivation and Context

The current implementation of the Device class assumes that the server response (servResp) is always non-nil before attempting to close its body. However, in cases where an error occurs and servResp is nil, calling servResp.Body.Close() will result in a runtime panic, which can cause the application to crash unexpectedly.

This pull request adds a nil check before attempting to close the server response body. This ensures that the connection is only reused if the server response is valid, thereby preventing potential runtime panics and improving the robustness of the code.

Changes Made

  • Added a nil check for servResp before calling servResp.Body.Close() in the callMethodDo and SendSoap methods.
  • This change is applied consistently across multiple instances in the Device.go file.

Why This Improves the Project

  1. Prevents Runtime Panics: By ensuring that servResp is non-nil before attempting to close its body, we prevent potential runtime panics that could crash the application.
  2. Improves Code Robustness: Adding these checks enhances the robustness and reliability of the code by handling edge cases more gracefully.
  3. Maintains Connection Reuse: The change ensures that the connection reuse logic is maintained without risking application stability.

Overall, this change contributes to the stability and reliability of the project, making it safer for production use.

@cedricve cedricve merged commit a7815a6 into master Jan 19, 2025
1 check passed
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.

1 participant