Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing own request.js to async interface #140

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

2colours
Copy link
Contributor

@2colours 2colours commented Sep 1, 2024

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.

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
Copy link
Contributor Author

2colours commented Sep 1, 2024

Note: this PR inevitably disregards #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
Copy link
Contributor Author

2colours commented Sep 10, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant