-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Improved list and feed errors #1798
Conversation
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, no blockers
export enum KnownError { | ||
FeedgenDoesNotExist, | ||
FeedgenMisconfigured, | ||
FeedgenBadResponse, | ||
FeedgenOffline, | ||
FeedgenUnknown, | ||
Unknown, | ||
} |
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.
Not a hill I'd die on, but I think we should only use string enums. These get evaluated to numbers, which makes rearranging difficult, etc.
export enum KnownError {
FeedgenDoesNotExist = 'FeedgenDoesNotExist'
...
}
Works 100% of the time, very clear.
feed.knownError === KnownError.Unknown | ||
) { | ||
return ( | ||
<ErrorMessage message={feed.error} onPressTryAgain={onPressTryAgain} /> |
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.
Hehe so we fall back to the red error?
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.
Hah ya but only for non-feedgens
Feedgen errors
List error state