Skip to content

Conversation

@johnkary
Copy link
Contributor

The other match() case is:

$query->connection->prepareBindings($query->bindings)

This method always returns an array.

Before this PR the match() case would return null when the softLimit had been exceeded, which is the wrong data type used downstream. This resulted in Exception:

Debugbar exception: foreach() argument must be of type array|object, null given

Background

I discovered this issue while debugging a Laravel application that executed a lot of queries. I saw that the phpdebugbar-id: Response header was being omitted on a specific Response that was executing the queries. After some digging I found the soft_limit number and increased it to just above my query count. The debug bar then appeared.

When tailing the application log I found the Exception:

Debugbar exception: foreach() argument must be of type array|object, null given

I used Xdebug to trace where in the library the Exception was thrown, and dove into the code to find the softLimit usage and eventually the type mismatch.

Let me know if you require unit tests for this change, but I didn't see any existing tests for this feature.

The other match() case is:
$query->connection->prepareBindings($query->bindings)

This method always returns an array.

Before this commit the match() case would return null, which is the wrong data type used downstream.
This resulted in Exception:

Debugbar exception: foreach() argument must be of type array|object, null given
@barryvdh barryvdh merged commit 02bdb05 into barryvdh:master Nov 13, 2024
33 checks passed
@johnkary johnkary deleted the soft-limit-exceeded-crash branch November 13, 2024 23:57
@barryvdh
Copy link
Owner

Thanks, this is pushed now. I added a browser test for this in #1703

@johnkary
Copy link
Contributor Author

@barryvdh Thank you very much for the quick merge and new release! I appreciate all your work on this library.

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.

2 participants