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

Better error messages #115

Merged
merged 9 commits into from
Apr 13, 2021
Merged

Better error messages #115

merged 9 commits into from
Apr 13, 2021

Conversation

cryptix
Copy link
Member

@cryptix cryptix commented Apr 1, 2021

fixes #66
fixes #23

Most importantly: I added errhandler.go and flashes.go to web/errors, these use localizeError() which translates a few hardcoded error messages to the users locale. I updated the english defaults accordingly.

Most of the handlers now have flashes weberrors.FlashHelper which has three methods:

  • AddMessage(rw http.ResponseWriter, req *http.Request, label string)
  • AddError(rw http.ResponseWriter, req *http.Request, err error)
  • GetAll(rw http.ResponseWriter, req *http.Request) ([]FlashMessage, error)

To render these there is a new base template flashes.tmpl which can be used with {{ tempalte "flashes" .}}, See the overviews of admin/members, admin/aliases, etc.

Noteworthy:

  • go.mindeco.de/http/[email protected] now handles cookies automatically which made it possible to simplify quite a bit of the testing code that uses login and csrf.

TODO:

  • style the flash messages (right now they are just red and green text)
  • flashify login errors
  • flashify notice edits

@cryptix cryptix added Go golang related stuff Testing everything that's testing related enhancement New feature or request labels Apr 2, 2021
@cryptix cryptix force-pushed the better-error-messages branch 4 times, most recently from 1157dc0 to e89de2a Compare April 12, 2021 08:39
@cryptix cryptix marked this pull request as ready for review April 12, 2021 11:57
@cryptix cryptix force-pushed the better-error-messages branch from c58d7a9 to 8ff4adf Compare April 12, 2021 12:10
@cryptix cryptix requested review from cblgh and staltz and removed request for cblgh April 12, 2021 14:39
but until then it might be nice to include a _previous_ data from the rendere with an url of where to go.
-->
<a href="javascript:history.back()" class="btn btn-primary">Back</a>
<a href="{{.BackURL}}" class="btn btn-primary">Back</a>
Copy link
Member

Choose a reason for hiding this comment

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

Yay!

Copy link
Member

@staltz staltz left a comment

Choose a reason for hiding this comment

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

The Go looks good, and I took a look at the styles, they seem simple and obvious, I don't think we need to improve the looks (an error anyway looks unpleasant, by design). What's important is that it's a consistent style, and that it clearly communicates an error.

@staltz
Copy link
Member

staltz commented Apr 12, 2021

Oh, and can other pages use the BackURL in the templates? I mean this as a question of possibility, not as something to be done within this PR.

@cryptix
Copy link
Member Author

cryptix commented Apr 13, 2021

Thanks! merging then :)

can other pages use the BackURL in the templates?

Not yet, right now it's handled in the errorhandler, which also looks at the status code to to a kinda-sane thing:

https://github.com/ssb-ngi-pointer/go-ssb-room/pull/115/files#diff-f49e5d91dc7f3e5f05f0b6b833a9b0a7b306612fae102bae8f7b95bb46170e86R55-R64

Where would you want to use it?

@cryptix cryptix merged commit 2ae16dd into master Apr 13, 2021
@cryptix cryptix deleted the better-error-messages branch April 13, 2021 07:03
@cblgh
Copy link
Contributor

cblgh commented Apr 13, 2021

🎉

@staltz
Copy link
Member

staltz commented Apr 13, 2021

Where would you want to use it?

Remove member confirmation, revoke invite confirmation, maybe other places.

@cryptix cryptix mentioned this pull request Apr 20, 2021
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Go golang related stuff Testing everything that's testing related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

localized error pages "flash" errors
3 participants