CCI-MOC / hil

Hardware Isolation Layer, formerly Hardware as a Service
Apache License 2.0
24 stars 54 forks source link

Make our error types more clear and useful #925

Open naved001 opened 6 years ago

naved001 commented 6 years ago

This stems from the discussion here

Errors we raise in our APIs should be distinct and clear enough so that we know how to process those errors just by looking at the type of it.

zenhack commented 6 years ago

Would be good to accumulate a list of cases that need dealing with. The one you mentioned:

Which also brings up the issue: in the latter case, the client might want to know what needs to be detached.

I can think of two general approaches off the top of my head:

It probably makes sense to combine these.

naved001 commented 6 years ago

It might make sense to e.g. split

@jeremyfreudberg suggested that we inherit from BlockedError. That way we could either catch the specific child exception, or the parent one if we don't care about the exact type.

zenhack commented 6 years ago

It's not obvious to me that it makes sense to have these two use cases have a shared ancestor. Can you think of a case where we'd want to handle BlockedError but not care about the difference above?

Also, worth noting that the class hierarchy doesn't get sent across the wire.

naved001 commented 6 years ago

Can you think of a case where we'd want to handle BlockedError but not care about the difference above?

Can't think of it now. Maybe in future, but in that case we might as well not do it.

Also, worth noting that the class hierarchy doesn't get sent across the wire.

Not sure what you mean by that? This code seems to work.

class ParentError(Exception):
    """Some Parent Error"""

class ChildError1(ParentError):
    """ChildError1"""

class ChildError2(ParentError):
    """different one"""

def raise_error1():
    raise ChildError1('Child1')

def raise_error2():
    raise ChildError2('Child2')

try:
    raise_error1()
except ParentError:
    # this will be printed
    print "Exception handled"
zenhack commented 6 years ago

Right, but the server sends them to the client as JSON, where the type is just a string, and it doesn't include information about what the superclasses are.

naved001 commented 6 years ago

Ah, that's what you meant. Let's simply split the errors like you said.

zenhack commented 6 years ago

Works for me.

RobinKaul commented 5 years ago

I am going to work on this so I wanted to know if there have been any other changes or if there are any suggestions apart from the ones above.

zenhack commented 5 years ago

Nothing jumps to mind; I think the proposed solution still makes sense.

The two suggestions should probably be separate prs, just to keep them small and self contained.