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

Errorpages #14967

Merged
merged 1 commit into from Sep 8, 2017

Conversation

Projects
None yet
5 participants
@holisticode
Contributor

holisticode commented Aug 11, 2017

This PR introduces simple HTML error pages for nicer error display in swarm. Closes #3447
It also fixes the issue of returning error code 404 instead of 500 if a manifest can't be found.

Show outdated Hide outdated swarm/api/api.go Outdated
Show outdated Hide outdated swarm/api/http/error.go Outdated
Show outdated Hide outdated swarm/api/http/error.go Outdated
Show outdated Hide outdated swarm/api/http/error.go Outdated
Show outdated Hide outdated swarm/api/http/error.go Outdated
Show outdated Hide outdated swarm/api/http/server.go Outdated
Show outdated Hide outdated swarm/api/http/server.go Outdated
Show outdated Hide outdated swarm/api/http/tmpl/htmlstrtmpl.go Outdated
Show outdated Hide outdated swarm/api/http/tmpl/htmlstrtmpl.go Outdated
Show outdated Hide outdated swarm/api/http/tmpl/htmlstrtmpl.go Outdated
Show outdated Hide outdated swarm/api/http/server.go Outdated
Show outdated Hide outdated swarm/api/http/tmpl/htmlstrtmpl.go Outdated
@GitCop

This comment has been minimized.

Show comment
Hide comment
@GitCop

GitCop Aug 15, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 64e1c79
  • Your commit message body contains a line that is longer than 80 characters

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

GitCop commented Aug 15, 2017

Thank you for your contribution! Your commits seem to not adhere to the repository coding standards

  • Commit: 64e1c79
  • Your commit message body contains a line that is longer than 80 characters

Please check the contribution guidelines for more details.


This message was auto-generated by https://gitcop.com

Show outdated Hide outdated swarm/api/api.go Outdated
"github.com/ethereum/go-ethereum/swarm/testutil"
)
func TestError(t *testing.T) {

This comment has been minimized.

@zelig

zelig Aug 16, 2017

Contributor

Do we not need more tests? maybe just unit test on the template map only?

@zelig

zelig Aug 16, 2017

Contributor

Do we not need more tests? maybe just unit test on the template map only?

This comment has been minimized.

@holisticode

holisticode Aug 17, 2017

Contributor

I agree that we can add more tests here, i.e. is the returned string a html page? Does any error code produce a html page? Other suggestions?

@holisticode

holisticode Aug 17, 2017

Contributor

I agree that we can add more tests here, i.e. is the returned string a html page? Does any error code produce a html page? Other suggestions?

This comment has been minimized.

@holisticode

holisticode Aug 17, 2017

Contributor

Added more tests

@holisticode

holisticode Aug 17, 2017

Contributor

Added more tests

Show outdated Hide outdated swarm/api/http/error.go Outdated
@zelig

please document all exported methods
plus 3 minor things in the comments

Show outdated Hide outdated swarm/api/http/server.go Outdated
Show outdated Hide outdated swarm/api/http/server.go Outdated
Show outdated Hide outdated swarm/api/http/server.go Outdated
@zelig

zelig approved these changes Aug 24, 2017

@fjl

This comment has been minimized.

Show comment
Hide comment
@fjl

fjl Sep 7, 2017

Contributor

Is this ready to merge?

Contributor

fjl commented Sep 7, 2017

Is this ready to merge?

@holisticode

This comment has been minimized.

Show comment
Hide comment
@holisticode

holisticode Sep 8, 2017

Contributor

@fjl yes from my side, not sure if it may need a rebase in the mean while?

Contributor

holisticode commented Sep 8, 2017

@fjl yes from my side, not sure if it may need a rebase in the mean while?

@zelig

This comment has been minimized.

Show comment
Hide comment
@zelig

zelig Sep 8, 2017

Contributor

@fjl ready yes

Contributor

zelig commented Sep 8, 2017

@fjl ready yes

@fjl fjl merged commit ac193e3 into ethereum:master Sep 8, 2017

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
commit-message-check/gitcop All commit messages are valid
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment