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

Unify and improve error reporting #35

Closed
gavv opened this issue Jan 10, 2017 · 9 comments
Closed

Unify and improve error reporting #35

gavv opened this issue Jan 10, 2017 · 9 comments
Labels
duplicate This issue or pull request already exists
Milestone

Comments

@gavv
Copy link
Owner

gavv commented Jan 10, 2017

  • Unify error formatting

    Currently, every component do its own error message formatting. Instead, all of them should fill a struct with error info and pass it to error formatter, which will format it as text and pass to error reporter. The user can provide their own error formatter.

  • Improve error messages

    The error message should include:

    • pretty-printed request
    • pretty-printed response
    • round-trip time
    • error message
    • pretty-printed expected value
    • pretty-printed actual value
    • diff
    • backtrace
  • Use colored output

    When running in a TTY, use colors for error message and diff (gojsondiff allows that). The user can enable, disable, or choose an automatic mode for this.

@gavv gavv added the feature New feature or request label Jan 10, 2017
@gavv
Copy link
Owner Author

gavv commented Jan 11, 2017

This would probably break API (the error reporter will be replaced with error formatter).

@gavv gavv added this to the v2 milestone Jan 11, 2017
@gavv gavv removed this from the v2 milestone Oct 21, 2017
@xwinie
Copy link

xwinie commented Feb 5, 2018

I expect have summary report page.

@gavv gavv added the help wanted Contributions are welcome label Apr 18, 2019
@gavv gavv removed the help wanted Contributions are welcome label Sep 25, 2019
@gavv
Copy link
Owner Author

gavv commented Dec 17, 2019

See also #62 and #35.

@gavv gavv added the help wanted Contributions are welcome label Dec 17, 2019
@goku321
Copy link
Contributor

goku321 commented Jan 17, 2020

I would be happy to help!

@gavv
Copy link
Owner Author

gavv commented Jan 17, 2020

@goku321 Great, you're welcome!

Please also take a look at these issues: #42, #59, #62. They're related enough.

It would be nice to do refactoring with those issues in mind. We can not only add report struct, but also add context struct, and pass both of them to error formatter.

So I suggest the following:

  • Introduce Context struct. Context is created in the root object and inherited by all child object (through the chain struct).

  • Introduce Report struct. Report is created when an assertion fails and contains info about the error.

  • Introduce Formatter. On error, context and report are passed to Formatter.

  • Formatter formats context and report into a (multi-line) string. It also implements features like pretty-printing, color formatting, diff, etc.

  • The string returned by Formatter is passed to Reporter (it already exists), which issues an assertion.

We first can introduce some minimal context, report, and formatter, and merge it. The we can incrementally add more info to context and reporter and improve formatter.

And after implementing this, we'll also be able to resolve the issues I linked above.

Formatter should be an interface to allow the user to substitute it. We should provide a default implementation, e.g. DefaultFormatter. We should allow the user to specify custom formatter via Config, and use default one if it's not specified.

BTW, This way we will not break the API.

@goku321
Copy link
Contributor

goku321 commented Jan 18, 2020

@gavv Thank you for the elaboration. I'll start with the issues you mentioned to get a better understanding.

@gavv
Copy link
Owner Author

gavv commented Apr 2, 2020

Closing with respect to #69.

@gavv gavv closed this as completed Apr 2, 2020
@gavv
Copy link
Owner Author

gavv commented Apr 2, 2020

@goku321 Are you still interested? I've prepared a more detail issue (linked above).

@gavv gavv added duplicate This issue or pull request already exists and removed help wanted Contributions are welcome feature New feature or request labels Apr 2, 2020
@gavv gavv added this to the v2 milestone Apr 2, 2020
@goku321
Copy link
Contributor

goku321 commented Apr 4, 2020

@gavv Yes, I am. But I can only start working on this from next week(if that's okay) as I am caught up with some stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists
Projects
None yet
Development

No branches or pull requests

3 participants