Missing request information in Builder rejected error

[masterT]

Currently, the Builder throws a generic Error from the AxiosError message when performing HTTP requests.

https://github.com/DavidDuwaer/Coloquent/blob/62e4823b9bb70e5244f59244a93c049872280c73/src/Builder.ts#L111-L113

Would there be a way to have access to the AxiosError? Some HTTP statuses are standard in the JSON:API protocol (https://jsonapi.org/format/#fetching-resources-responses-404). Maybe this should be reflected in the library too as custom Error classes.

My case is that I want the HTTP status, so I can determine if the resource exists on the server.

[DavidDuwaer]

I don't have time at the moment, but

Would there be a way to have access to the AxiosError?

There certainly should, in my opinion. If it's not possible, this should be fixed.

Some HTTP statuses are standard in the JSON:API protocol (https://jsonapi.org/format/#fetching-resources-responses-404). Maybe this should be reflected in the library too as custom Error classes.

I am for that.

I can take a look later, but if you feel like adding the axios error on the coloquent error and make a PR then I'll try to merge and release that asap so you can get ahead.

[masterT]

Thank for the fast feedback, I can help with the implementation.

I see a lot of AxioError type being used in the codebase, should this be encapsulated by the HttpClient interface? It feels like it would not be compatible if there is another concert HttpClient.

[DavidDuwaer]

should this be encapsulated by the HttpClient interface? It feels like it would not be compatible if there is another concert HttpClient.

Could you elaborate? On 7 Jan 2022, 13:01 +0100, Simon Thiboutôt @.***>, wrote:

Thank for the fast feedback, I can help with the implementation. I see a lot of AxioError type being used in the codebase, should this be encapsulated by the HttpClient interface? It feels like it would not be compatible if there is another concert HttpClient. — Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android. You are receiving this because you commented.Message ID: @.***>

[masterT]

I meant "another concrete HttpClient class", for example having another HttpClient that would use the native fetch API instead of Axios.

class FetchHttpClient implements HttpClient {
  get(url: string): HttpClientPromise {
    const init = { /* ... */ }
    return new FetchHttpClientPromise(fetch(url, init));
  }

  // ...
}

Using this client won't be compatible with the code that refers to AxiosError, wouldn't it?

https://github.com/DavidDuwaer/Coloquent/blob/ac27390a234333881cdeecdb51c532613f8b1240/src/Model.ts#L228

I just want to point out that I'm not a TypeScript expert. 😅

[DavidDuwaer]

Ok, yeah, we encapsulating them seems like the solution for that! I suppose we need to convert those AxiosErrors before they bubble out of the (implementation of the) HttpClient's methods (rather than after, as e.g. in Builder:62), into something like this:

class HttpError extends Error
{
    source: Error | undefined

    constructor(message: string, source?: Error) {
        this.source = source;
    }
}

and then the AxiosError or any concrete HttpClient's error can be appended to it as the source property, as they are would all be subclasses of Error