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

feat: enhanced error messages for cloud and oss specific commands #347

Merged
merged 2 commits into from
Dec 28, 2021

Conversation

williamhbaker
Copy link
Contributor

@williamhbaker williamhbaker commented Dec 23, 2021

Closes #142

This will use the actual X-Influxdb-Build header returned by the server to determine the cause for an error.

The mechanism for flagging commands as cloud/OSS only is also improved. Middleware is used to add information to the cli.Context for host-specific commands, and an exit error handler middleware determines if a host-specific command was used, and if the wrong host type is present in the response header. This should make it much easier to keep track of which commands are specific to which host.

@williamhbaker
Copy link
Contributor Author

Example error messages

Cloud-only commands run on OSS:

New:

influx bucket-schema list --bucket-id 1234123412341234
Error: InfluxDB Cloud-only command used with InfluxDB OSS host

Old:

influx bucket-schema list --bucket-id 1234123412341234
Error: failed to find bucket "1234123412341234": InfluxDB Cloud-only command failed: 404 Not Found: bucket not found

OSS-only commands run on Cloud:

New:

influx ping       
Error: InfluxDB OSS-only command used with InfluxDB Cloud host

Old:

influx ping
Error: InfluxDB OSS-only command failed: unable to decode response content type "text/html"

New:

influx remote list
Error: InfluxDB OSS-only command used with InfluxDB Cloud host

Old:
(remote was not listed as an OSS-only command, although it should have been)

influx remote list             
Error: failed to get remote connections: 404 Not Found: path not found

Copy link
Contributor

@mcfarlm3 mcfarlm3 left a comment

Choose a reason for hiding this comment

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

Took me a bit of time to trace my way through this, but it looks good to me! A lot cleaner than before, and this will be a good pattern to have for adding any future OSS-only or Cloud-only commands.

@williamhbaker
Copy link
Contributor Author

Renamed the test TestMain_HostSpecificErrors to TestApp_HostSpecificErrors to avoid confusion with the actual TestMain function prior to merging.

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.

Improve error messages even more for OSS- or cloud-only commands
2 participants