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

Ability to get the last response #359

Merged
merged 2 commits into from
May 18, 2021
Merged

Ability to get the last response #359

merged 2 commits into from
May 18, 2021

Conversation

ejunker
Copy link
Contributor

@ejunker ejunker commented May 16, 2021

Fixes #358

Added a getSyncClient() method to Querier and added a getResponse() method to SyncClient. This allows you to access the last response including the headers.

I did not add anything to AsyncClient since I wasn't sure how that would work since it returns a Promise.
I wasn't sure the best way to update the tests so let me know if I should change anything. I wanted to test that the response is still set when there is an exception but couldn't figure out how to test that with the mocked objects.

…o SyncClient. This allows you to access the last response including the headers.
@atymic atymic requested a review from reliq May 17, 2021 07:39
Copy link
Collaborator

@reliq reliq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @ejunker, only a few suggestions.

Thank you for contributing 😄

src/Contract/Http/SyncClient.php Outdated Show resolved Hide resolved
tests/Unit/Service/QuerierTest.php Outdated Show resolved Hide resolved
src/Contract/Querier.php Show resolved Hide resolved
@reliq reliq self-assigned this May 17, 2021
@ejunker
Copy link
Contributor Author

ejunker commented May 17, 2021

@reliq made the suggested changes. Renamed getResponse() to getLastResponse() and added getAsyncClient()

@atymic atymic merged commit 1ca2b9c into atymic:main May 18, 2021
@atymic
Copy link
Owner

atymic commented May 18, 2021

Thanks for the PR @ejunker and thanks for the review @reliq :)

@ejunker ejunker deleted the get-response branch May 18, 2021 12:54
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.

Access response headers to get x-rate-limit header values
3 participants