Skip to content

Undefined index: Content-Type in LogAPIResponseToLog #236

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

Closed
kirill-mally opened this issue Jul 17, 2019 · 14 comments
Closed

Undefined index: Content-Type in LogAPIResponseToLog #236

kirill-mally opened this issue Jul 17, 2019 · 14 comments

Comments

@kirill-mally
Copy link

kirill-mally commented Jul 17, 2019

At some point, begin getting error notice about Undefined index: Content-Type in HttpClients/SyncRestHandler.php.

Environment:

  • macOS High Sierra
  • PHP 7.2.13 (cli) (built: Dec 7 2018 10:41:55)
  • Mostly tested in Chrome browser

Root of the problem is in incorrect cast of the constant Content-Type variable.
I'm not sure whether it depends on the PHP settings or cast conversion has to be done manually but had to add fix on my end to proceed.

So, when the strings compare was made, we are calling the array item but with capitalized letters. Firstly, I had to transform characters into lowercase to match the corresponding array key. Secondly, cover the whole piece of code into try-catch logic for not letting Exception outside and ruining the respond.

    /**
     * Log API Reponse to the Log directory that user specified.
     * @param String $body The requestBody
     * @param String $requestUri  The URI for this request
     * @param Array $httpHeaders  The headers for the request
     */
    public function LogAPIResponseToLog($body, $requestUri, $httpHeaders) {
        try {
            $contentType = strtolower(CoreConstants::CONTENT_TYPE);
            if (
                strcasecmp($httpHeaders[$contentType], CoreConstants::CONTENTTYPE_APPLICATIONXML) == 0
                || strcasecmp($httpHeaders[$contentType], CoreConstants::CONTENTTYPE_APPLICATIONXML_WITH_CHARSET) == 0
            ) {
                $body = $this->parseStringToDom($body);
            }
        } catch (\Exception $e) {
            $body = "";
        }

        $this->RequestLogging->LogPlatformRequests($body, $requestUri, $httpHeaders, false);
    }

Comment on this case and if it makes sense and worth adding to the next release, please do it. I'm worried that after another composer update my edits will be lost.

Thanks.

@kirill-mally
Copy link
Author

It's strange.

Can you please make sure that you have single LogAPIResponseToLog function?

I'm using the latest version of SDK but on my end, i have already another LogAPIRequestToLog() function on line 296.

@JavaBog
Copy link

JavaBog commented Jul 17, 2019

It's strange.

Can you please make sure that you have single LogAPIResponseToLog function?

I'm using the latest version of SDK but on my end, i have already another LogAPIRequestToLog() function on line 296.

You're correct, it works now. I replaced the second of the two functions. It works replacing the first only.

@kirill-mally
Copy link
Author

kirill-mally commented Jul 17, 2019

Glad it worked!

Note that one you replaced is a Respond and another is Request.
And my only concern is the value that i'm declaring to a variable in case of Exception catch. As much as i've checked code further, return an empty string should be fine for future XML concatenations.

If you're more familiar with the native library, please do check this moment:

        } catch (\Exception $e) {
            $body = "";
        }

Thanks for the support.

@msankar1991
Copy link

I'm getting same issue with version 5.0 itself.

@virendrachandak
Copy link

I have the same issue with the 5.0.4 version. I don't see the duplicate function mentioned above. Editing the function as mentioned by @kirill-mally fixes the issue. Thanks.

@tariqwiztech
Copy link

I have upgraded to 5.0.4 version but didn't find the duplicate function. Tried @kirill-mally solution and its works but modifying core files is not a solution.

@kirill-mally
Copy link
Author

@tariqwiztech I'm not aware of how to add the fix to the core function and commit it here.
That's why i've created an issue and would like to involve somebody responsible for this branch.
If i run an update of composer on a live server apparently it will throw a mistake there too.

That's why it will be nice if maybe @hlu2 could note about the problem.
Sorry for bothering but it appeared to be a problem because the original code didn't allow me to proceed after establishing the OAuth2 connection.

@diana-derose
Copy link
Collaborator

Hi - Can one of you please provide the company id for which you are getting the incorrect headers?

@kirill-mally
Copy link
Author

@diana-derose For us it's the Eljin Productions Inc. company.
But that should not be the case.

I believe that problem lies in what fields or params were used in DataService object creation.
At first i thought that problem was solved after removing the received companyInfo string:

$CompanyInfo = $dataService->getCompanyInfo();

But the problem persisted on the OAuth authentication process.
Also, seems that it do not depend on the environment used. Had same behavior for development and production versions. Because there is no try-catch logic, it didn't catch any exception and code broke at some point by throwing an error notice.

I've got a respond from @hlu2:

V3 Team have a configuration that changes some realmIDs from returning “Content-Type” to “content-type”. I will do a patch and release now.

@virendrachandak
Copy link

I am just using sandbox for development and it is happening there. It is happening when I try to create an invoice or even when trying to connect.

@diana-derose
Copy link
Collaborator

thanks @kirill-mally and @virendrachandak for additional details. Unfortunately, we are not able to reproduce this on our end but @hlu2 will add the fix now so all of you are unblocked.

@kirill-mally
Copy link
Author

Thanks @diana-derose and thanks @hlu2 once again for a rapid response.
Great that it was solved so quickly!

hlu2 added a commit that referenced this issue Jul 18, 2019
@hlu2
Copy link
Contributor

hlu2 commented Jul 18, 2019

Fixed.

@hlu2 hlu2 closed this as completed Jul 18, 2019
@tariqwiztech
Copy link

Thanks @hlu2. Its fixed now :)

alexjeen added a commit to vontis-nl/QuickBooks-V3-PHP-SDK that referenced this issue Jul 23, 2019
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

No branches or pull requests

7 participants