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

Proposal: new error page #1776

Closed
rstacruz opened this issue Jun 27, 2016 · 21 comments
Closed

Proposal: new error page #1776

rstacruz opened this issue Jun 27, 2016 · 21 comments

Comments

@rstacruz
Copy link
Contributor

rstacruz commented Jun 27, 2016

Hey, friends. Let's talk about Phoenix's error page. I think we can do a lot of improvement here.

image

Situation

This design has served Phoenix and better_errors well for a while, but it can use some improvement.

  • The exception messages are limited to one line (tho more info can be shown on hover). This is kind of cumbersome, considering many Elixir errors can span multiple lines.
  • The request info/headers/params/session area (below the source) made sense for better errors, because better_errors allowed you to inspect what's in each stack frame. Not quite so in Phoenix. It feels weird that it's huge and it looks like it changes when you click on a stack frame on the left.
  • It feels needlessly verbose. Most times I'm only concerned about what error happened, and the line that triggered it. These need to be presented more prominently.

Proposal

How about something more like this?

image

Gone is the weird frame-like scrolling; instead, it scrolls down naturally like a page would. (except for the stack trace... you should be able to scroll it down on its own iframe-like container.)

You'll see that above the fold, most the relevant things are what immediately draws your attention.

image

By putting all the "more info" stuff at the bottom, we get more space to do more interesting things. How links to search Google for the error? Or maybe link to the relevant hexdocs?

image

This is a rough draft of course, but if you like where this is going I'd be happy to polish it up and implement it.

By the way

I'm the author of the original better_errors error page, which I believe Phoenix's is based off of :)

@josevalim
Copy link
Member

This is so elegant, thank you! ❤️ 💚 💙 💛 💜

The exception messages are limited to one line (tho more info can be shown on hover). This is kind of cumbersome, considering many Elixir errors can span multiple lines.

This has been my biggest pet peeve and you solved it perfectly! I absolutely love the links to search and docs too. Well done!

Btw, the debug pages come from Plug. Here is the source: https://github.com/elixir-lang/plug/blob/master/lib/plug/debugger.ex and here is the template: https://github.com/elixir-lang/plug/blob/master/lib/plug/templates/debugger.eex

I would merge this PR in a heartbeat. :) Let me know if there is anything I can do to help!

@josevalim
Copy link
Member

josevalim commented Jun 27, 2016

I'm the author of the original better_errors error page, which I believe Phoenix's is based off of :)

Yes! I emailed you 3 years ago to ask for your blessing. :D

@rstacruz
Copy link
Contributor Author

oh man I'm so sorry I didn't see your email on that, haha, but it's all MIT license so it's all good!

@rstacruz
Copy link
Contributor Author

(Oh, you may have emailed Charlie @charliesome—he's the author of better_errors, I just contributed frontend work to his project)

@josevalim
Copy link
Member

@rstacruz No problems! I e-mailed you both but went ahead because of MIT anyway. :D

@chrismccord
Copy link
Member

chrismccord commented Jun 27, 2016

@rstacruz this looks amazing! I love every change you're proposing. Your work on better_errors is something I include in every talk I give about having "pretty errors", and this would take it over the top :)

We'd love to have this in place if you're up for it. Since this is on the plug side, we'll need to think how to best get this in plug, but also allow it to be extensible so we can include the help links that are dep specific. Thanks!

@rstacruz
Copy link
Contributor Author

Neat! What would be the best way to put together a little development setup to hack Plug.Debugger? (I'm guessing maybe something like a mini-phoenix app with deps/plug being part of elixirc_paths and Phoenix.CodeReloader reloadable_paths?)

@rstacruz
Copy link
Contributor Author

but also allow it to be extensible so we can include the help links

After giving this some thought, I realized that feature might have been thinking way too far ahead—for a first iteration, maybe something that just implements the baseline functionality that the current version has would be best.

@chrismccord
Copy link
Member

yes that sounds like a good firs step in plug. With some hooks to plug debugger I think we could make it extensible, but getting baseline in place first is a good start

@josevalim
Copy link
Member

And if we decide to go ahead with the links anyway, I can help with making them configurable. :)

@josevalim
Copy link
Member

@rstacruz if you clone both phoenix and plug locally and depend on plug as {:plug, path: "../path/to/plug"}, I believe it should recompile it automatically. If not, you can start Phoenix with iex -S mix phoenix.server and call r Plug.Debugger whenever you change the template.

@rstacruz
Copy link
Contributor Author

okay, that's quick and easy. thanks for the tips!

@allyraza
Copy link

allyraza commented Jul 2, 2016

@rstacruz if you need help I can certainly lend you a hand

@rstacruz
Copy link
Contributor Author

hmm, r Plug.Debugger isn't quite reloading template changes for me.

@josevalim
Copy link
Member

@rstacruz I will investigate soon and provide a small Plug app that does code reloading only for Plug.Debugger. :D

@rstacruz
Copy link
Contributor Author

haha, cool, i'm doing ctrl+c - up - enter for now :D

@josevalim
Copy link
Member

@rstacruz if you create a file named app.exs at the root of your Plug checkout with the following contents:

defmodule MyRouter do
  use Plug.Router
  use Plug.Debugger

  plug :reload
  plug :match
  plug :dispatch

  defp reload(conn, _) do
    IEx.Helpers.r(Plug.Debugger)
    conn
  end

  get "/oops" do
    _ = conn
    raise "oops"
  end
end

{:ok, _} = Plug.Adapters.Cowboy.http MyRouter, port: 4000

You can start it with mix run --no-halt app.exs and now you access localhost:4000/oops for a custom exception or any route for a function clause error. Live reloading should work (although it is a hack :D).

@rstacruz
Copy link
Contributor Author

rstacruz commented Jul 12, 2016

Thanks for everyone who's volunteering to help! I have it working on this branch, feel free to try it out and break it (maybe with @josevalim's test code up there). Just a few more refinements and I'll open up a PR. PR opened (elixir-plug/plug#423).

plugtest

@rstacruz
Copy link
Contributor Author

@josevalim
Copy link
Member

@rstacruz yes, that would definitely be the next step. I have changed Phoenix master to depend on Plug 1.2-dev in case you want to give it a try inside Phoenix.

@josevalim
Copy link
Member

Actually, we still need to change the code reloader error pages. So I am reopening this. :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants