A basic error handler

[curquiza]

We should add a basic error handler, with these 2 custom errors:

• MeiliSearchApiError
• MeiliSearchCommunicationError

If I'm not wrong, @barbados-clemens you created one in your previous meilisearch-dotnet repo (not available anymore) 🙂 Was this complicated to add?

FYI: Even if it's not documented yet, MeiliSearch API has a great error handler, and returns this kind of JSON when an error is found:

It could be great to let the user access message, errorCode, errorType and errorLink.

[barbados-clemens]

I was just following the logic of how the js library was structured.

Here is the JS HTTP error handler for reference

I didn't implement those error handlers correctly just more stubbed out, as I wasn't sure how to go about deciding what falls into those types of errors.

But we can make two classes that extend Exception or a specific HTTP exception, such as HttpResponseException for the MeiliSearchCommunicationError which my understanding is for when there's a 5xx error code

The other is for when there's an issue in the action the user performed, which is when we'd get meilisearch specific errors back like in the image you showed above, i.e. 4xx based codes.

the Error code I wrote was basically this

public class MeiliSearchApiError : Exception 
{
    // probably have the properties as shown via the image above that meilisearch returns 
    public MeiliSearchApiError(){}
    public MeiliSearchApiError(string message){}
    public MeiliSearchApiError(string message, Exception innerException){}
    // not sure what else we'd want or need for this class
}

public class MeiliSearchCommunicationError : Exception // or we could do HttpResponseException
{
   public MeiliSearchCommunicationError() {}
   // whatever else we'd need
}

public static class ErrorHandler 
{
   public static void HandleHttpError(Exception ex, string cachedStack // don't think we need this?)
   {
      // 4xx error, the user sent in a bad request and meilisearch returns us a helpful error object
      if(isClientSideError) 
      {
         throw new MeiliSearchApiError(//whatever we want to include)
      }

     // 5xx error that we can't communicate to the search insance
      if(isServerSideError) 
      {
         throw new MeiliSearchCommunicationError(//whatever we want to include)
      }

     throw ex    
   }
}

Once again I have no idea if this is the correct way to do this as I was just following based on the guidance of the js library.

Would love to hear some feedback on if this is the direction we'd want to go or not.

[curquiza]

I was just following the logic of how the js library was structured.

That's a good solution! The JS library should follow these guidelines 🙂 The python and ruby one implement it too!

But we can make two classes that extend Exception or a specific HTTP exception, such as HttpResponseException for the MeiliSearchCommunicationError which my understanding is for when there's a 5xx error code

Hum there is a miss-understanding... The CommunicationError is there to be thrown if we cannot reach the server, I mean no response at all (even a 5XX or 4XX).

ex:

MeilisearchClient client = new MeilisearchClient("http://localhost:8800", "masterKey"); // you can notice that is not the right port (that should be 7700)
var index = await client.CreateIndex("movies"); // throw a MeiliSearchCommunicationError

In the situation where the meilisearch-dotnet library can reach the server, it would throw a MeiliSearchApiError.

MeilisearchClient client = new MeilisearchClient("http://localhost:7700", "masterKey"); 
var index = await client.CreateIndex("movies"); // throw a MeiliSearchApiError if the index "movies" (404) does not exist or if there is an internal MeiliSearch error (5XX)

Maybe the naming is confusing and that's why you miss-interpreted this way... What do you think? Your point of view could be really interesting because we (the Meili team) chose these names for the basic errors but maybe they are not really explicit. With new suggestions, we can re-think the naming of our error handlers (for all the SDKs) 🙂

[barbados-clemens]

That makes sense and the errors match what the happening. My brain just jumped to status codes instead of slowing down the thinking about what they actually mean.

So for a CommunicationError that would happen if we can't connect to the search instance. An ApiError is when meilisearch is able to send us the error, i.e. can connect but user send a bad request or something went wrong with processing the request on search instance side.

The ApiError is pretty straight forward I believe we just check for status code >= 400 and throw the MeilisearchApiError with the details of the response. I presume meilisearch doesn't return an error payload that is 3xx or 2xx.

as for the CommunicationError, when looking into how the HttpClient handles this type of error (i.e. given the wrong address/can't connect), this is the trace I get back

System.Net.Http.HttpRequestException: No connection could be made because the target machine actively refused it.
 ---> System.Net.Sockets.SocketException (10061): No connection could be made because the target machine actively refused it.
   at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
   --- End of inner exception stack trace ---
   // more trace that doesn't really matter...

so we can just catch this specific error HttpRequestException and transform it into a ~MeiliSearchApiException~ MeiliSearchCommunicationError to throw with some helpful messages.

[curquiza]

The ApiError is pretty straight forward I believe we just check for status code >= 400 and throw the MeilisearchApiError with the details of the response. I presume meilisearch doesn't return an error payload that is 3xx or 2xx.

Exactly! 🙂

so we can just catch this specific error HttpRequestException and transform it into a MeiliSearchApiException to throw with some helpful messages.

You mean MeiliSearchCommunicationException, right?

Otherwise, if you agree with that, all seem ok from my POV to start a basic error handler 🙂

[barbados-clemens]

Oops!

Yes I meant MeiliSearchCommunicationError. Fixed that in my previous comment.

What would this look like?

my first thought was going and wrapping everything into a try/catch but that's a great way to duplicate code.

So maybe an extension method or wrapper method to execute the call and handle the exception logic?

we could centralize all the HTTP calls instead of executing them directly in the methods

public async Task<T> GetDocument<T>(string documentId)
        {
            return await this.HttpGetWrapper<T>($"/indexes/{Uid}/documents/{documentId}");
        }

public async Task<T> HttpGetWrapper<T>(string url)
        {
            try
            {
                var res = await this._client.GetAsync(url);
                // res.IsSuccessStatusCode is 200-299, not sure if we need to handle 3xx codes, 
                // if not then we can use the built it check
                if ((int) res.StatusCode >= 400)
                    throw new MeiliSearchApiError(res.Content);

                // where Parse, parses the JSON
                return Parse<T>(res.Content);
            }
            catch (HttpRequestException httpError)
            {
                throw new MeiliSearchCommunicationError(httpError);
            }
        }

or something like that, I'm just tossing an idea out there.

[curquiza]

Of course, we have to avoid the try/catch in every method (getDocuments, createIndex...etc), so we indeed need a wrapper.

Don't know if it can help for the implementation, but in our meilisearch-python we have an HTTPRequest class which is a wrapper for all the HTTP methods.

See: https://github.com/meilisearch/meilisearch-python/blob/master/meilisearch/_httprequests.py

Plus, we succeeded to avoid code duplication even in the get, post, put and delete methods. Maybe we can apply this kind of implementation in this package?