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 formatting of javascript errors #339

Closed
wants to merge 2 commits into from
Closed

Conversation

aiba
Copy link
Contributor

@aiba aiba commented Aug 28, 2021

I think in javascript land, we want to show the stringified error (which contains the type of error and err.message, both important pieces of info) as well as the stacktrace if it exists.

@ptaoussanis
Copy link
Member

Looks good, thanks Aaron! Am hoping to get a chance to cut a new release this weekend. Cheers :-)

@ptaoussanis ptaoussanis self-assigned this Aug 30, 2021
@ptaoussanis
Copy link
Member

Merging manually now, thanks again (and apologies for the delay!)

@DerGuteMoritz
Copy link
Contributor

Just happened upon this issue. Since the code comment still asks for alternatives, I would like to mention the formatting function we've been using quite successfully so far:

(defn format-error [err]
  (str (.-stack err) ; includes `ex-message`
       (when-let [d (ex-data err)]
         (str "\nex-data\n    " (pr-str d)))
       (when-let [c (ex-cause err)]
         (str "\ncaused by - " (format-error c)))))

As you can see, it also walks the cause chain and prints ex-data.

@aiba
Copy link
Contributor Author

aiba commented Sep 27, 2022

@DerGuteMoritz yeah this function is even better. Do you want to submit a PR for this, or if you'd like I can copy/paste it into my PR.

@aiba
Copy link
Contributor Author

aiba commented Sep 27, 2022

Oh I just realized my PR was already merged. In any case, this function seems like an improvement, perhaps @DerGuteMoritz you can submit a PR with it?

@DerGuteMoritz
Copy link
Contributor

Sure thing, here you go! Adjusted it to also use enc/system-newline.

ptaoussanis pushed a commit that referenced this pull request Oct 18, 2022
ptaoussanis pushed a commit that referenced this pull request Oct 19, 2022
ptaoussanis pushed a commit that referenced this pull request Oct 20, 2022
ptaoussanis pushed a commit that referenced this pull request Oct 22, 2022
…eMoritz)

Changes incl.:

  - Print ex-data
  - Print full cause chain
ptaoussanis pushed a commit that referenced this pull request Oct 23, 2022
…eMoritz)

Changes incl.:

  - Print ex-data
  - Print full cause chain

Changes will affect all Cljs users of the default output fn (and/or the default
error fn), i.e. most Cljs users.

Since the effect of these changes is just additional information in logging output,
my hope is that the change should be a positive or neutral one for the vast majority
of users.

To customise error formating, see the `default-output-fn` docstring.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants