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

Consistently use Error::NotAvailable instead of Unsupported #349

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

joeshaw
Copy link
Member

@joeshaw joeshaw commented Mar 12, 2024

This PR switches most uses of Error::Unsupported to Error::NotAvailable, and translates both to FastlyStatus::Unsupported.

Unsupported is meant to express things that are not supported by the platform, for example trying to make a request with "HTTP/4".

NotAvailable is meant to express things that are not available or unimplemented in Viceroy.

Previously, Unsupported would return FastlyStatus::Unsupported and NotAvailable would return the generic FastlyStatus::Error. This led to a bad user experience for the NotAvailable case, as the user would see an opaque and generic "Error" message rather than indicating an operation was "Unsupported".

This PR (1) changes Error::NotAvailable to map to FastlyStatus::Unsupported and (2) changes all our uses of Error::Unsupported for things that are not implemented in Viceroy to be Error::NotAvailable.

joeshaw added 2 commits March 12, 2024 13:47
`Unsupported` is meant to express things that are not supported by the
platform, for example trying to make a request with "HTTP/4".

`NotAvailable` is meant to express things that are not available or
unimplemented in Viceroy.

Previously, `Unsupported` would return `FastlyStatus::Unsupported` and
`NotAvailable` would return the generic `FastlyStatus::Error`.  This
led to a bad user experience for the `NotAvailable` case, as the user
would see an opaque and generic "Error" message rather than
"Unsupported".  The previous commit fixes this by having both
`Unsupported` and `NotAvailable` return `FastlyStatus::Unsupported`.

This commit makes our use of `NotAvailable` consistent.
Copy link
Contributor

@cee-dub cee-dub left a comment

Choose a reason for hiding this comment

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

LGTM (though I'm pretty new to Rust)

@joeshaw joeshaw merged commit 97a67fa into main Mar 13, 2024
7 checks passed
@joeshaw joeshaw deleted the joeshaw/not-available-features branch March 13, 2024 12:38
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.

3 participants