-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: expose HTTP response headers in InfluxDBApiException #118
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
+ Coverage 95.81% 95.83% +0.02%
==========================================
Files 12 12
Lines 1098 1104 +6
Branches 141 143 +2
==========================================
+ Hits 1052 1058 +6
Misses 11 11
Partials 35 35 ☔ View full report in Codecov by Sentry. |
Client/InfluxDBApiException.cs
Outdated
|
||
public HttpResponseHeaders GetHeaders() | ||
{ | ||
return HttpResponseMessage?.Headers; | ||
} | ||
|
||
public HttpStatusCode GetStatusCode() | ||
{ | ||
return HttpResponseMessage?.StatusCode ?? 0; | ||
} |
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.
This could be written using property getters (invocations in tests need to be modified accordingly), is that right, @bednar?
In any case, Headers
/GetHeaders
return value should be nullable (T?
)
public HttpResponseHeaders GetHeaders() | |
{ | |
return HttpResponseMessage?.Headers; | |
} | |
public HttpStatusCode GetStatusCode() | |
{ | |
return HttpResponseMessage?.StatusCode ?? 0; | |
} | |
public HttpResponseHeaders? Headers | |
{ | |
get | |
{ | |
return HttpResponseMessage?.Headers; | |
} | |
} | |
public HttpStatusCode StatusCode | |
{ | |
get | |
{ | |
return HttpResponseMessage?.StatusCode ?? 0; | |
} | |
} |
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.
Thanks. A Sensible suggestion. Updated accordingly.
Client.Test.Integration/WriteTest.cs
Outdated
"Could not parse entire line. Found trailing content: 'distance=,status=\"STOPPED\"'" | ||
)); | ||
Assert.That(iaex.GetStatusCode().ToString(), Is.EqualTo("BadRequest")); | ||
Assert.That(iaex.GetStatusCode().GetHashCode(), Is.EqualTo(400)); |
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.
There is System.Net enum for status code, eg.
Assert.That(iaex.GetStatusCode().GetHashCode(), Is.EqualTo(400)); | |
Assert.That(iaex.GetStatusCode(), Is.EqualTo(System.Net.HttpStatusCode.BadRequest)); |
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.
Updated, following suggestion.
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.
LGTM
Proposed Changes
InfluxDBApiException
to get StatusCode and Headers from nestedHttpResponseMessage
more easily.IntegrationTest
class.InfluxDBApiException
General/Runner.cs
class for unifying running examples.HttpErrorHandled.cs
.Checklist