search

pulsar-edit / ppm

Pulsar Package Manager
MIT License
35 stars 13 forks source link

Changing own request.js to async interface #140

Open 2colours opened 2 weeks ago

2colours commented 2 weeks ago

This is the version that passes all the existing tests. The HTTP requests used to be handled by a callback that had three arguments: one for the errors if there are any, one for the response and one for the payload within the response. Now, the error has been turned into rejected promises (exceptions when awaited) and the payload has simply been dropped. It's extracted from the response via the .body attribute.

2colours commented 2 weeks ago

Note: this PR inevitably disregards https://github.com/pulsar-edit/ppm/pull/139. I mean, it wouldn't absolutely have to but they don't really add up. Many of the code paths that PR joined together ("either error, or some status code") would be completely unrelated code paths following the usual logic of superagent. As described on the discord server, I would propose something like a validator callback passed as an option to the request calls. If the predicate is false, an error should be raised, rather than the response returned. For the current logic, even a dummy error (eg. a null value) would suffice but obviously there is a lot of space for being clever here.

2colours commented 1 week ago

As described on the discord server, I would propose something like a validator callback passed as an option to the request calls. If the predicate is false, an error should be raised, rather than the response returned. For the current logic, even a dummy error (eg. a null value) would suffice but obviously there is a lot of space for being clever here.

Oh yeah... apparently, we are already doing something like that, with OK_STATUS_CODES and the ok method in the superagent call chain. I think sometimes (often, even) it would be good to allow the user to pass the list of acceptible status codes themselves, at least. OK_STATUS_CODES should be more of a default value.