Better error handling

[praveenperera]

I think the error handling good be improved a little bit. Instead of always returning the same RPCError struct we could pattern match on the status code and return different errors. Currently I'm doing it like below. If you like this approach I could do a PR:

defmodule Bossman.Job.Error do
  defmodule NotFound, do: defexception([:message])
  defmodule Unknown, do: defexception([:message])
  defmodule InvalidArgument, do: defexception([:message])

  def new(%GRPC.RPCError{message: message, status: status}) do
    status_to_error_struct(status, URI.decode(message))
  end

  defp status_to_error_struct(2, msg), do: %Unknown{message: msg}
  defp status_to_error_struct(3, msg), do: %InvalidArgument{message: msg}
  defp status_to_error_struct(5, msg), do: %NotFound{message: msg}
end

[polvalente]

@praveenperera would you still like to contribute with this?

I think this approach is good and we can accompany it with a is_rpc_error guard so users can still match in a generic manner wherever needed.

[polvalente]

Another possible solution is to just document the status atoms better

[hackvan]

Another possible solution is to just document the status atoms better

Hi @polvalente,

Do you think its enough adding an ASCII table in the @moduledoc like this:

Error	Status	Status Message
0	:ok
1	:cancelled	The operation was cancelled (typically by the caller)
2	:unknown	Unknown error
3	:invalid_argument	Client specified an invalid argument
4	:deadline_exceeded	Deadline expired before operation could complete
5	:not_found	Some requested entity (e.g., file or directory) was not found
6	:already_exists	Some entity that we attempted to create (e.g., file or directory) already exists
7	:permission_denied	The caller does not have permission to execute the specified operation
8	:resource_exhausted	Some resource has been exhausted
9	:failed_precondition	Operation was rejected because the system is not in a state required for the operation's execution
10	:aborted	The operation was aborted
11	:out_of_range	Operation was attempted past the valid range
12	:unimplemented	Operation is not implemented or not supported/enabled in this service
13	:internal	Internal errors
14	:unavailable	The service is currently unavailable
15	:data_loss	Unrecoverable data loss or corruption
16	:unauthenticated	The request does not have valid authentication credentials for the operation

Would it be better to include it in the GRPC.Status module or in the GRPC.RPCError module?

[polvalente]

@hackvan I think that's perfect. We can put that in GRPC.Status and add something like "See GRPC.Status for more details on possible statuses" in GRPC.Error

[polvalente]

I think we could provide this macro in GRPC.RPCError together with the better documentation:

defmacro is_rpc_error(e, status) when is_struct(e, GRPC.RPCError) and e.status == status

Which would make it easier for users to pattern match on different error types without the hassle of dealing with many different kinds of structs

[hackvan]

@hackvan I think that's perfect. We can put that in GRPC.Status and add something like "See GRPC.Status for more details on possible statuses" in GRPC.Error

What do you think?

I think we could provide this macro in GRPC.RPCError together with the better documentation...

For this I don't get it what is the purpose or a case use for this macro 😅

[polvalente]

That looks good, but I'd format it with a markdown table so ex doc renders it nicely.

The guard (I wrote defmacro but should have been defguard there) covers the use case mentioned in the issue body itself, where instead of having different structs for pattern matching, we can just use the guard for matching on different errors

[hackvan]

That looks good, but I'd format it with a markdown table so ex doc renders it nicely.

Yeap, I did it that way:

The guard (I wrote defmacro but should have been defguard there) covers the use case mentioned in the issue body itself, where instead of having different structs for pattern matching, we can just use the guard for matching on different errors

Ok, I get it but in this case when the people need to use the defguard they would need to import the GRPC.RPCError module first, right?

[polvalente]

Yes. Or require it. But that's a lot more manageable than aliasing N different structs