Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Clean up apierrors and only include messages #1329

Merged
merged 2 commits into from
May 27, 2020

Conversation

jspspike
Copy link
Contributor

@jspspike jspspike commented May 27, 2020

Makes API errors a little cleaner. Looked like this before
Error: [ApiError { code: 10037, message: "user is missing required permission \"#worker:edit\"", other: {} }]

Now it looks like
Error: Code 10037: user is missing required permission "#worker:edit" Edit your API Token to have correct permissions, or use the 'Edit Cloudflare Workers' API Token template.

@jspspike jspspike added this to the 1.10.0 milestone May 27, 2020
@jspspike jspspike requested a review from a team as a code owner May 27, 2020 19:39
@jspspike
Copy link
Contributor Author

Addresses #1157

.iter()
.map(|e| e.message.clone())
.collect();
failure::bail!("ApiError {:?}", error_messages);
Copy link
Contributor

Choose a reason for hiding this comment

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

oh actually - it looks like we already have a convenience function for this that should make it easier + uniform with the way we display errors.

It is crate::http::format_error, and it takes an ApiFailure, and an optional function to handle individual error codes.

So you could write something real quick to include the suggestions for codes 10026 and 10014 and pass it to that function. You can see an example of this in crate::src::commands::route::delete, at the end there is a match result and it calls http::format_error

This should allow us to display the error without making it look like an array

Copy link
Contributor

Choose a reason for hiding this comment

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

(if i had thought of this before i would have pointed you in that direction!)

Copy link
Contributor

@EverlastingBugstopper EverlastingBugstopper left a comment

Choose a reason for hiding this comment

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

lgtm!

@EverlastingBugstopper
Copy link
Contributor

added a changelog - feature label to this one

@EverlastingBugstopper EverlastingBugstopper merged commit ab92e2b into master May 27, 2020
@delete-merged-branch delete-merged-branch bot deleted the josh/permission-message branch May 27, 2020 21:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants