-
Notifications
You must be signed in to change notification settings - Fork 195
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
Decoding empty body resulted in syntax error #2668
base: 3.x
Are you sure you want to change the base?
Conversation
596be08
to
902287a
Compare
902287a
to
710f113
Compare
Build error says: "The maximum number of Multidev environment for this service level has been reached." 😿 |
JSON_THROW_ON_ERROR | ||
); | ||
} catch (\JsonException $jsonException) { | ||
$this->logger->debug($jsonException->getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should prepend a string in front of jsonException->getMessage() to make this line easier to find should it happen again in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100% agree, and $body
should not overwrite $body
on line 434. we should preserve the body before attempting json decode, and print what was in it if the json decode fails.
|
||
// Don't attempt to decode JSON if the response is expected to have no body. | ||
if ( | ||
!in_array($statusCode, [204, 304], true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be helpful to be more explicit (i.e., explain in a comment) the significance of these particular status codes, and why this check is important to do in addition to the content-length / empty body checks.
$body = null; | ||
} | ||
} else { | ||
$body = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why force null?
Previously when in verbose mode, Terminus would report
[debug] Syntax error
at the end of a request.When there is no body in the response, there's no need to attempt to decode it. We set it to null here.