3

I am currently working on an integration with Personio to fetch employee data.

Instead of waiting for a response, the solution below suddenly jumps out of the second method at the point commented in the code:

using Personio.Client.Configuration;
using Personio.Client.Exceptions;
using Personio.Client.Models.Data;
using LanguageExt;
using Newtonsoft.Json;
using RestSharp;

namespace Enpal.Personio.Client;

public class PersonioCompanyEmployeesClient
{
    private readonly PersonioAuthorizationClient _authorizationClient;
    private readonly PersonioConfig _personioConfig;
    private readonly RestClient _restClient;

    public PersonioCompanyEmployeesClient(PersonioAuthorizationClient authorizationClient, PersonioConfig personioConfig, RestClient restClient)
    {
        _authorizationClient = authorizationClient;
        _personioConfig = personioConfig;
        _restClient = restClient;
    }

    public TryAsync<List<Record>> TryGet(EmployeeRequestOptions options) =>
        _authorizationClient.WithAuthorization<List<Record>>(token =>
        {
            _restClient.BaseUrl = new Uri(_personioConfig.BaseUrl, "/v1/company/employees");
            _restClient.Timeout = -1;

            IRestRequest request = new RestRequest(Method.GET)
                .AddQueryParameter("limit", options.MaximumRecordCount.ToString())
                .AddQueryParameter("offset", options.PageOffset.ToString())
                .AddHeader("Accept", "application/json")
                .AddHeader("Authorization", $"Bearer {token.Value}");

            return GetRecordsAsync(request);
        });

    private async Task<List<Record>> GetRecordsAsync(IRestRequest request)
    {
        IRestResponse<RecordRequestResponse> requestResponse = await _restClient.ExecuteAsync<RecordRequestResponse>(request);
        // Nothing below this line is ever executed!
        if (requestResponse.IsSuccessful)
        {
            RecordRequestResponse? employeesResponse = JsonConvert.DeserializeObject<RecordRequestResponse>(requestResponse.Content);
            return (employeesResponse != null && employeesResponse.WasSuccessful)
                ? employeesResponse.Records
                    .ToList()
                : throw new PersonioRequestException("Connected to Personio, but could not get records.");
        }
        else
        {
            throw (requestResponse.ErrorException != null)
                ? new PersonioRequestException("Could not get records from Personio.", requestResponse.ErrorException)
                : new PersonioRequestException("Could not get records from Personio.  Reason unknown.");
        }
    }
}

However, I can make this solution work by using the synchronous method, like so:

public TryAsync<List<Record>> TryGet(EmployeeRequestOptions options) =>
        _authorizationClient.WithAuthorization<List<Record>>(token =>
        {
            _restClient.BaseUrl = new Uri(_personioConfig.BaseUrl, "/v1/company/employees");
            _restClient.Timeout = -1;

            IRestRequest request = new RestRequest(Method.GET)
                .AddQueryParameter("limit", options.MaximumRecordCount.ToString())
                .AddQueryParameter("offset", options.PageOffset.ToString())
                .AddHeader("Accept", "application/json")
                .AddHeader("Authorization", $"Bearer {token.Value}");

            return GetRecordsAsync(request);
        });

    private async Task<List<Record>> GetRecordsAsync(IRestRequest request)
    {
        IRestResponse<RecordRequestResponse> requestResponse = await _restClient.ExecuteAsync<RecordRequestResponse>(request);
        if (requestResponse.IsSuccessful)
        {
            RecordRequestResponse? employeesResponse = JsonConvert.DeserializeObject<RecordRequestResponse>(requestResponse.Content);
            return (employeesResponse != null && employeesResponse.WasSuccessful)
                ? employeesResponse.Records
                    .ToList()
                : throw new PersonioRequestException("Connected to Personio, but could not get records.");
        }
        else
        {
            throw (requestResponse.ErrorException != null)
                ? new PersonioRequestException("Could not get records from Personio.", requestResponse.ErrorException)
                : new PersonioRequestException("Could not get records from Personio.  Reason unknown.");
        }
    }

The only changes:

  1. GetRecords() uses Execute instead of ExecuteAsync
  2. GetRecords() returns List<Record> instead of Task<List<Record>>
  3. TryGet() wraps GetRecordsAsync(request) in Task.FromResult() before returning it.
ProgrammingLlama
  • 36,677
  • 7
  • 67
  • 86
Brian Kessler
  • 2,187
  • 6
  • 28
  • 58

3 Answers3

1

After looking at the restsharp's documentation, ExecuteAsync does not throw an exception, but instead populates response.ErrorException and response.ErrorMessage if the response.IsSuccessful equals to false so you should check first for the response.IsSuccessful property value.

More details can be found here - https://restsharp.dev/intro.html#content-type

Ran Turner
  • 14,906
  • 5
  • 47
  • 53
  • 1
    The failure is silent. You are correct, no exception is thrown. However, nothing below the indicated point (including the check for success) is ever executed. Immediately after the invocation, the system jumps out of the method and the consuming code tells me the value returned by `TryGet` is null. I feel like the `await` is just being ignored. – Brian Kessler Dec 16 '21 at 16:12
  • Did you check both ErrorException and ErrorMessage properties? – Ran Turner Dec 16 '21 at 16:16
  • Not possible. Even with a breakpoint on the line `IRestResponse requestResponse = await _restClient.ExecuteAsync(request);`. If I try to step over or step into that line, I'm immediately out of that method to somewhere that requestResponse no longer exists. – Brian Kessler Dec 16 '21 at 16:28
  • Did you try to wrap your code try catch blocks? – Ran Turner Dec 16 '21 at 16:29
  • I have the debugger set to break when absolute anything is thrown (including things that are not default), but nothing is thrown. – Brian Kessler Dec 16 '21 at 16:37
  • Please try to wrap your code with try catch blocks and share your information once again – Ran Turner Dec 16 '21 at 16:39
0

I must add that RestSharp's async requests were messy until the last version. You can have a quick look at how HttpWebRequest handles async calls, and what RestSharp did to manage them, it's hardcore. I know that it worked for some, but definitely, the issues are there.

If it's still relevant for you, I would advise moving to v107. Those interfaces you use are gone, not sure how big of an issue it would be for you. But also all sync methods are gone and it's fully async now, and it works.

Alexey Zimarev
  • 17,944
  • 2
  • 55
  • 83
  • Looks like there are some large breaking changes... not just removing sync, but also removing interfaces and removing support for generic parameters?!?! I want to try this solution, but can't prioritize that big a change right now... putting this on backburner. – Brian Kessler Jan 21 '22 at 10:59
  • Not sure what you mean by removed support for generic parameters tbh. – Alexey Zimarev Jan 21 '22 at 13:26
  • For example, I have this line `IRestResponse> requestResponse = _restClient.Execute>(request);` which won't compile... To get it to compile, I can make it `RestResponse requestResponse = await _restClient.ExecuteAsync(request, cancellationToken);` (or something like that), but then I have to deal handle that lost Type information differently. – Brian Kessler Jan 21 '22 at 15:55
  • Not sure I understand what is the issue with `var requestResponse = _restClient.ExecuteAsync>(request);` Everything is documented. – Alexey Zimarev Jan 21 '22 at 21:21
  • I'll have to try again later, but I remember having compilation issues and then clicking around and not seeing any methods with type parameters. Maybe I missed something. – Brian Kessler Jan 23 '22 at 01:00
0

I had the exact same problem and it turns out the reason was because I had a synchronous call hidden in my chain. So this line: return GetRecordsAsync(request); needs to have an await, which means TryAsync should be an asynchronous call as well. I have run into similar problems in the past where not having async from top to bottom causes issues. Easiest solution is make everything async, though there are other possible solutions as well

EGP
  • 616
  • 5
  • 11