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

feat(gateway): improved templates, user friendly errors #298

Merged
merged 1 commit into from
May 30, 2023
Merged

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented May 12, 2023

Closes #262. This PR does the following:

  • Adds a new human-friendly error page when something wrong goes on.
  • Reworks templates a bit, making the process to develop faster and easier.
    • Common header file.
    • No more bash scripts or go generate.
    • Cleaned up CSS.
    • Removed twitter and opengraph meta tags and just use standard <title> and <meta description.
  • Updated some file icons for the directory to be coherent.
  • Removed "Preview as JSON" from DAG template. We removed that function in fix(gateway): undesired conversions to dag-json and friends kubo#9566, but forgot to update the template.
  • Removed hardcoded Kubo issues page and replaced by customisable menu items in the config. Each implementation can now add their own links that will show up to the right of "About" and "Install".

As usual, instructions on how to test the templates are in gateway/assets/README.md.

image image

@hacdias hacdias self-assigned this May 12, 2023
@codecov
Copy link

codecov bot commented May 12, 2023

Codecov Report

Merging #298 (ab987b2) into main (86cdb24) will decrease coverage by 1.18%.
The diff coverage is 44.04%.

❗ Current head ab987b2 differs from pull request most recent head 37da207. Consider uploading reports for the commit 37da207 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #298      +/-   ##
==========================================
- Coverage   49.37%   48.20%   -1.18%     
==========================================
  Files         279      281       +2     
  Lines       33598    33646      +48     
==========================================
- Hits        16590    16220     -370     
- Misses      15276    15737     +461     
+ Partials     1732     1689      -43     
Impacted Files Coverage Δ
gateway/gateway.go 88.52% <ø> (ø)
gateway/handler_block.go 0.00% <0.00%> (-74.36%) ⬇️
gateway/handler_car.go 0.00% <0.00%> (-58.34%) ⬇️
gateway/handler_codec.go 0.00% <0.00%> (-32.95%) ⬇️
gateway/handler_ipns_record.go 0.00% <0.00%> (-20.69%) ⬇️
gateway/handler_tar.go 0.00% <0.00%> (-55.13%) ⬇️
gateway/handler_unixfs__redirects.go 37.63% <0.00%> (+1.07%) ⬆️
gateway/handler_defaults.go 36.66% <11.11%> (+0.24%) ⬆️
gateway/assets/test/main.go 20.37% <26.82%> (ø)
gateway/handler_unixfs_dir.go 63.58% <30.00%> (+2.08%) ⬆️
... and 5 more

... and 13 files with indirect coverage changes

@hacdias
Copy link
Member Author

hacdias commented May 12, 2023

I'm opening this for review. Please give me feedback regarding error messages and other things.

@hacdias hacdias marked this pull request as ready for review May 12, 2023 11:16
@hacdias hacdias requested a review from lidel as a code owner May 12, 2023 11:16
Copy link
Contributor

@BigLep BigLep left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. A few things:

  1. This needs a changelog entry.
  2. How does someone customize this for their own needs? 3 immediate examples I see:
  • (most important) how does ipfs.io gateway customize this for their product
  • (maybe) Kubo default (which can encourage someone to install Kubo for themselves)
  • (likely not as important) bifrost-gateway default

gateway/assets/error.html Outdated Show resolved Hide resolved
gateway/assets/error.html Outdated Show resolved Hide resolved
gateway/assets/error.html Outdated Show resolved Hide resolved
gateway/assets/error.html Outdated Show resolved Hide resolved
gateway/assets/src/page-header.html Outdated Show resolved Hide resolved
gateway/assets/error.html Outdated Show resolved Hide resolved
gateway/assets/error.html Outdated Show resolved Hide resolved
gateway/assets/src/error.html Outdated Show resolved Hide resolved
gateway/assets/src/error.html Outdated Show resolved Hide resolved
gateway/assets/src/error.html Outdated Show resolved Hide resolved
gateway/errors.go Outdated Show resolved Hide resolved
@hacdias hacdias force-pushed the issue/262 branch 3 times, most recently from 01d1085 to fd662bc Compare May 25, 2023 07:55
@hacdias
Copy link
Member Author

hacdias commented May 25, 2023

@BigLep

How does someone customize this for their own needs?

Very good question. And I think the answer depends on what you want to customize. What do you need to customize? Right now none of the templates are or have been customizable (directory index, DAG index, and now error). I removed the Kubo hardcoded link and added customisable links for the menu.

@hacdias hacdias force-pushed the issue/262 branch 2 times, most recently from ece7d2a to 202f364 Compare May 25, 2023 08:30
@hacdias hacdias force-pushed the issue/262 branch 7 times, most recently from b34d3c7 to 699bbc3 Compare May 25, 2023 15:29
@hacdias hacdias requested a review from lidel May 25, 2023 15:30
@hacdias hacdias changed the title feat(gateway): user friendly error messages feat(gateway): improved templates, user friendly errors May 26, 2023
@hacdias hacdias force-pushed the issue/262 branch 3 times, most recently from d215a1b to 00428cd Compare May 26, 2023 12:29
@hacdias hacdias force-pushed the issue/262 branch 3 times, most recently from 557b1ba to d4f57e7 Compare May 26, 2023 13:23
@hacdias hacdias requested a review from a team as a code owner May 26, 2023 13:23
@hacdias hacdias force-pushed the issue/262 branch 2 times, most recently from ab987b2 to 110e584 Compare May 29, 2023 08:19
gateway/assets/error.html Outdated Show resolved Hide resolved
Copy link
Member

@lidel lidel left a comment

Choose a reason for hiding this comment

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

@hacdias thank you for cleaning this up 🙏

Mind resolving conflicts and creating a PR draft in Kubo before merging?
If Kubo is green, please merge and rebase #307 so we can continue there 🙏

gateway/assets/README.md Outdated Show resolved Hide resolved
gateway/assets/dag.html Outdated Show resolved Hide resolved
@hacdias
Copy link
Member Author

hacdias commented May 30, 2023

Checks are green in Kubo: ipfs/kubo#9904. Merging.

@hacdias hacdias enabled auto-merge (squash) May 30, 2023 09:41
@hacdias hacdias merged commit 0cfdf85 into main May 30, 2023
@hacdias hacdias deleted the issue/262 branch May 30, 2023 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

gateway: return user-friendly HTML with HTTP errors
3 participants