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: support influxdb v1.8 HTTP error response message #134

Merged

Conversation

alxndrdude
Copy link
Contributor

@alxndrdude alxndrdude commented Jan 25, 2024

An influxdb v1.8 HTTP error response message was not correctly transferred to ruby InfluxError message. Hence, in case of an InfuxDB v1.8 Error response, the reason (except status code) was not traceable.

Closes #133

Proposed Changes

After parsing the response body of an InfuxDB Error response, the information of it is wrapped into an InfluxDB2::InfluxError.
Evaluation of the error message has to be different for InfluxDB v1.8 and InfuxDB v2.x, because the response body is different.

How the fix works:

  • InfluxDB v2.x uses the message key for this
  • InfluxDB v1.8 uses the error key for this

-> previously only message was supported
-> now, in case message is blank, error is checked

Checklist

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • rake test completes successfully
  • Commit messages are conventional
  • Sign CLA (if not already signed)

@alxndrdude alxndrdude changed the title Fix #133 - support influxdb v1.8 HTTP error response message fix: support influxdb v1.8 HTTP error response message Jan 29, 2024
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @alxndrdude,

Thank you for your contribution and the effort you've put into this Pull Request 👍. It's great to see your involvement and the value you're adding to the project.

Before we can proceed with merging your PR, please ensure that all items on our checklist are satisfied. This step is crucial for maintaining the quality and consistency of the codebase. Once you've completed the checklist, we'll be more than ready to move forward with merging your changes.

If you have any questions about the checklist or need further clarification on any of the points, feel free to reach out.

image

Best Regards

@alxndrdude alxndrdude force-pushed the fix-133-error-rsp-msg-influxdb-v1 branch from bca01b1 to 41a7bf6 Compare January 31, 2024 07:03
@codecov-commenter
Copy link

codecov-commenter commented Jan 31, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9eb5d8b) 48.23% compared to head (53df47b) 48.23%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #134   +/-   ##
=======================================
  Coverage   48.23%   48.23%           
=======================================
  Files          77       77           
  Lines        7948     7948           
=======================================
  Hits         3834     3834           
  Misses       4114     4114           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

An influxdb v1.8 HTTP error response message was not correctly transfered to the ruby InfluxError message.
In case of an InfuxError, the reason (error message) was not traceable.

How the fix works:
- InfluxDB v1.8 and v2.x provide Error details in the json-rsp body
- InfluxDB v2.x uses the `message` key for this
- InfluxDB v1.8 uses the `error` key for this

-> previously only `message` was supported
-> now, in case `message` is empty, `error` is checked

Refs: influxdata#133
@alxndrdude alxndrdude force-pushed the fix-133-error-rsp-msg-influxdb-v1 branch from 41a7bf6 to 53df47b Compare January 31, 2024 07:23
Copy link
Contributor

@bednar bednar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for your PR 👍

LGTM 🚀

@bednar bednar merged commit d5de6c9 into influxdata:master Jan 31, 2024
22 checks passed
@bednar bednar added this to the 3.1.0 milestone Jan 31, 2024
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.

An influxdb v1.8 HTTP error response message is not correctly transfered to InfluxError message
3 participants