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

Make possibility to extend CurlClient #1088

Merged
merged 3 commits into from
Mar 22, 2021
Merged

Make possibility to extend CurlClient #1088

merged 3 commits into from
Mar 22, 2021

Conversation

masterjus
Copy link
Contributor

@CLAassistant
Copy link

CLAassistant commented Feb 8, 2021

CLA assistant check
All committers have signed the CLA.

@remi-stripe
Copy link
Contributor

@masterjus Thanks for the PR! We'll have a look and get back to you if we need more changes!

@richardm-stripe
Copy link
Contributor

richardm-stripe commented Feb 10, 2021

Hi @masterjus, thanks for the submission. This seems reasonable to me but I don't want to merge without some idea of how concretely this would be used. Would you mind writing a little bit about your use case and the context for extending CurlClient?

@richardm-stripe
Copy link
Contributor

Hi @masterjus, closing for now but feel free to reopen if you can explain some more about the use case for this.

@masterjus
Copy link
Contributor Author

Hi @richardm-stripe

We have our own API which communicates with the Stripe's API and we want to log every request/response to Stripe.

So we have overridden method request to do log it automatically:

<?php
    public function request($method, $absUrl, $headers, $params, $hasFile)
    {
        $this->logger()->logRequest(collect([
            'method' => $method,
            'url' => $absUrl,
            'headers' => $headers,
            'body' => $params,
        ]));

        list($response['body'], $response['code'], $response['headers']) =
            parent::request($method, $absUrl, $headers, $params, $hasFile);

        $this->logger()->logResponse(collect($response));

        return array_values($response);
    }

If there will be some events which I could listen to, it would be awesome.

@masterjus
Copy link
Contributor Author

@richardm-stripe, could you please restart checks? Previous failed because code coverage wasn't sent to coveralls.io.

@richardm-stripe richardm-stripe merged commit e3195e2 into stripe:master Mar 22, 2021
@richardm-stripe
Copy link
Contributor

Merging. GHA is new/experimental and Travis is passing.

@ftrudeau-pelcro
Copy link

@masterjus Care to share more complete code on how you extended request method exactly ? We'd also like to log incoming and outgoing traffic via Stripe's API. Thanks :-)

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.

None yet

5 participants