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

[Tracking] Better errors #107

Closed
wants to merge 12 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@talex5
Contributor

talex5 commented Mar 12, 2016

The mirage tool currently generates setup code that ignores the actual errors returned by the connect functions and prints a generic error. This PR tracks changes to mirage-types and to the various libraries to allow mirage to produce useful error messages. For example:

Before:

    Fatal error: exception Failure("block11")

After:

    *** Setup failure ***

    Error connecting block device "disk.img":
      block-unix: connecting "/home/user/work/cubieboard/skeleton/block/disk.img":
      Unix.Unix_error(Unix.ENOENT, "open", "/home/user/work/cubieboard/skeleton/block/disk.img")
@samoht

This comment has been minimized.

Show comment
Hide comment
@samoht

samoht Mar 18, 2016

Member

Great work! As we've discussed in Marrakesh, I think the only thing that I don't really like in these patch is the fact that users have to explicitly wrap their job with with_logs. I'd feel much happier if it was an optional argument with a default log reporter, that we can enable/disable on the command-line line. Other than that I'm very happy with the rest of the patches! :-)

Member

samoht commented Mar 18, 2016

Great work! As we've discussed in Marrakesh, I think the only thing that I don't really like in these patch is the fact that users have to explicitly wrap their job with with_logs. I'd feel much happier if it was an optional argument with a default log reporter, that we can enable/disable on the command-line line. Other than that I'm very happy with the rest of the patches! :-)

@talex5

This comment has been minimized.

Show comment
Hide comment
@talex5

talex5 Mar 27, 2016

Contributor

The attribute thing is easy to change. We need to wait for some solution to be accepted into Functoria first though:

mirage/functoria#55

Contributor

talex5 commented Mar 27, 2016

The attribute thing is easy to change. We need to wait for some solution to be accepted into Functoria first though:

mirage/functoria#55

@samoht

This comment has been minimized.

Show comment
Hide comment
@samoht

samoht Jul 14, 2016

Member

I'm not super happy with error_string too, would be great if we had pp_error instead?

Member

samoht commented Jul 14, 2016

I'm not super happy with error_string too, would be great if we had pp_error instead?

@talex5 talex5 referenced this pull request Jul 14, 2016

Closed

[Tracking] Better errors #559

@yomimono yomimono referenced this pull request Jul 27, 2016

Closed

pending items for consideration for Mirage 3.0 #571

9 of 9 tasks complete
@hannesm

This comment has been minimized.

Show comment
Hide comment
@hannesm

hannesm Oct 2, 2016

Member

closing in favour of the merged mirage/mirage#602 (and #155 in here). would be great to get the error types updates massaged into mirage-types and implementation libraries!

Member

hannesm commented Oct 2, 2016

closing in favour of the merged mirage/mirage#602 (and #155 in here). would be great to get the error types updates massaged into mirage-types and implementation libraries!

@hannesm hannesm closed this Oct 2, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment