Debugging (PHP) Applications
PHP Benelux, 28. 1. 2018
© David Buchmann, Liip AG
twitter.com/bramcohen/status/51714087842877440
Organisation
- Introduction
- Tutorial Application
- Bug Hunting
- --- break ---
- More Bug Hunting
- Bug Prevention
- Bug Detection
- Wrap-Up
Introduction
Debugging
- Methodical guessing
- Narrowing down the problem
- Quickly validate a guess
Security Bugs
- Missing access control
- Functionality exposed: e.g. eval(), exec(),
string concatenation for SQL queries
- Missing validation of input
This workshop focuses on finding the location of bugs. Scanning for security vulnerabilities is worth its own Workshop.
Bug Report
Look at the list of open bugs if already reported
Title
Platform/Environment
What did you do - how to reproduce?
What did you expect to happen?
What was the actual result?
Screenshot if visual, copy console output if text (use ``` in github)
For OSS: Pull request with failing test, or even a fix?
Respect!
Demo Tutorial Application
- Import data from CSV into a relational database
- Doctrine ORM models
- Website that shows imported data
Bug Hunting
Usage
git reset --hard
git checkout [branchname]
If you get random errors about files in
/vagrant/var/cache/dev/ not found, do:
rm -rf var/cache/dev/
I often use obvious errors to illustrate a case. Real bugs will often be more complicated, but the strategies apply...
[b1] Bug 1
Look at exception message
[b1] RTFEM: Wrong Typehint
Cannot autowire service "App\Importer\Likes": argument "$em" of method "__construct()" references class "Doctrine\ORM\EntityManager" but no such service exists. Try changing the type-hint to one of its parents: interface "Doctrine\O
RM\EntityManagerInterface", or interface "Doctrine\Common\Persistence\ObjectManager".
- The exception message suggests a solution
- Try that suggestion
✔ Exceptions are your friends, read them carefully.
[b2] Bug 2
bin/console app:import
Argument -v makes output more verbose.
Look at the stack trace.
[b2] Stacktrace
App\Importer\Mapper\LikeMapper->map() at .../Likes.php:49
App\Importer\Likes->import() at .../AppImportCommand.php:60
App\Command\AppImportCommand->execute() at .../Command.php:252 ...
- Figure out what we are calling
- Check where the data comes from
- Check what would make sense
$this->productRepository->find($data['CustomerId']);
✔ Exception stack trace provides context of errors
[b3] DDD: Die Driven Debugging
bin/console app:import
The CustomerMapper checks if customer id already exists.
Add
die('cache')|found|new in CustomerMapper::map
[b3] DDD
$customer = $this->customerRepository->find($data['ProductId']);
- Ok if you have a confident guess
- Quick way to ensure you look at the correct place
- Stop after second attempt though
- ProTipâ„¢ do not commit the die() to git
✔ At most, use die() to check if you are in the expected place of the code
[b4] Use proper exceptions
bin/console app:import
Search code for
"oops"
Search for
die( and throw exceptions with decent messages
[b4] Use proper exceptions
- An exception should tell what happened
- Not overly verbose
- But provide enough context to be useful
- ! Careful with user input / large data
✔ do not make your collegues and your future self hate you
Demo: Step by step debugger
; config to run the remote debugger
xdebug.remote_enable=1
xdebug.remote_autostart=1
xdebug.remote_connect_back=1
xdebug.idekey="XDEBUG-DRIFTER"
Demo: Step by step debugger
- Configure your IDE
- Set IDE to listen to debugging
- ifconfig to find IP of vboxnet1
- php -d xdebug.remote_host=10.10.10.1 ./bin/console app:import
- https://blog.theodo.fr/2016/08/configure-xdebug-phpstorm-vagrant/
[b5] Segfault/Infinite Loops
bin/phpunit
At which test does it hang? Try --debug
[b5] Infinite Loop
Last line says CsvProductLoaderTest::testLoadFileNotFound
✔ progressive report to see which test fails hard
[b6] Search codebase for context
http://debugging.lo
Look at the page source for some context, then search the codebase to see what happens around it.
[b6] Search codebase for context
<h2 style="color:green;">Found {{ customer_count }} customers {{ render(controller('App\\Controller\\HomeController::extra')) }}
- Set a breakpoint in HomeController::extra
✔ Program output can provide context where to look for errors
[b7] Search in logfiles
bin/console app:import --reset -q
http://debugging.lo shows only 99 customers instead of 100
Search for non-debug messages in logfile:
echo '' > var/log/dev.log
bin/console app:import --reset -q
cat var/log/dev.log | grep -v DEBUG
[b7] Search in logfiles
app.WARNING: Invalid number of columns for App\Importer\Loader\CsvLikeLoader at CSV row 26
- Look at the CSV file data/likes.csv
- Line 26 has a comma in the name and no quotes
- Tell provider of this CSV to fix their thing
✔ Search logfiles for id's or message fragments. It helps if you know when the problem happened.
[b8] Unit Test (for debugging)
We built a (horrible) security into our website. But its faulty:
http://debugging.lo/?secret=I%20am%20David
http://debugging.lo/?secret=David
Build a unit test for the HomeController::index method to cover all cases.
[b8] Unit Test
if (!stripos($secret, 'david')) {
- Use false === stripos() and 0 === stripos() to prevent errors
- (And never do security like this! Use a form login or basic auth, read up on token authentication.)
✔ Build a unit test when debugging a specific component. Often you want to keep that test afterwards.
[b9] Do not assume
bin/phpunit
all green, great?
run bin/console app:import
The unit test CustomerMapperTest got it wrong with mocking.
[b9] Do not assume
CustomerMapperTest
$this->repository
->expects($this->once())
->method('find')
- ->with('1607080270099')
+ ->with('Quyn Goff')
->will($this->returnValue(null));
- Mocking in unit tests can hide errors
- We even have an integration test
- Unfortunately it does not recreate the specific situation
✔ Be wary of mocking.
[ba] git bisect
http://debugging.lo
bin/phpunit   But it used to work!!
https://www.sumologic.com/blog/devops/who-broke-my-test-a-git-bisect-tutorial/
[ba] git bisect
git bisect start
git bisect bad ba # broken version
git bisect good ba-1.0 # known working version
# => now you are in the middle of the history
# repeat
# run bin/phpunit and see if it goes through
# if so
git bisect good
# else
git bisect bad
[ba] git bisect
git show 09f8efd68cd535fc670c687081a77a5c4a32f0e8
- if ('\App\Entity\Customer' !==
- $this->options['customer_class']
+ if (Customer::class !==
+ $this->options['customer_class']
) {
die();
- Bonus points: What is the problem?
✔ If you know the last good state and have an easy way to decide between good and bad, git bisect is very efficient at localizing a problematic commit.
[bb] Symfony debug toolbar
http://debugging.lo
New feature: Only show expensive products
- Do we have expensive products in the CSV/database?
- What query has been run?
Symfony Debug Toolbar or tail var/log/dev.log
[bb] Symfony debug toolbar
WHERE p0_.price > 10 AND p0_.price < 10
- Somebody has been tired...
✔ Look at the actual SQL query that has been executed.
[bb] Logging SQL
- Set up doctrine dbal logging
- Use a PHP aware profiler (blackfire.io, tideways.io)
- Enable query logging in MySQL (beware, this will be verbose):
SET GLOBAL log_output = "FILE";
SET GLOBAL general_log_file = "/path/logfile.log";
SET GLOBAL general_log = 'ON';
See also: Slow query log
[bc] How did this ever work?
bin/phpunit
- This clearly does not do what was expected
- You are sure it did work on the server
Visit 3v4l.org and run the suspicious bit of code.
[bc] How did this ever work?
http://php.net/manual/en/migration70.incompatible.php
#migration70.incompatible.strings.hex
✔ If you suspect PHP version issues, 3v4l.org is a great tool to quickly check.
[bd] Using linters
- We added .travis.yml for automated testing.
- travis-ci complains about the file, can't handle it.
- What is wrong with the file?
Symfony provides the
lint:yaml command
[bd] Using linters
- Indentations in YAML are hard
- Not all tools provide good error messages when syntax errors occur
- Alternatively, there are web sites like https://jsonformatter.org/yaml-validator
✔ Linting helps to find syntax errors.
[be] Demo: Performance issue
«Premature optimization is the root of all evil.» (Donald Knuth)
- "Something" is slow
- Before we look at any code, use a tool to identify what is slow
php -d xdebug.profiler_enable=1 bin/console app:import
(xdebug.profiler_output_dir must exist, it is not created automatically)
KCacheGrind
[be] Demo: Performance issue
Alternatives:
- xhprof (for PHP 7, use https://github.com/tideways/php-profiler-extension) and a frontend for xhprof data.
- SaaS like blackfire.io (or tideways.io, newrelic)
✔ Find the slow part rather than optimize random bits of code.
[bf] Memory issue
bin/console app:import --reset
Start logging memory_get_usage() (or use blackfire.io)
[bf-fixed] Memory issue
We flush customers in batches
- PHP request/response cycle often avoids problem
- Give up references to let garbage collector work
- Beware of naive array caches
- https://tideways.io/profiler/blog/how-to-optimize-the-php-garbage-collector-usage-to-improve-memory-and-performance
✔ Memory issues are tricky, they often depend on the data. Avoid keeping unnecessary references.
Other Strategies
- Potential vendor bugs: composer install --prefer-source
- Google generic part of error message
- Ask collegues, ask on twitter (3v4l.org, gist)
Also: Rubberducking
- Take a break
Bug Prevention
Software Lifecycle
- Conception
- Design
- Development
- Testing
- Release
(Agile too, just not as isolated phases)
How to prevent errors
- Peer review (meaningful chunks)
- Pair programming
- (External) code reviews
- Unit testing
- Functional / integration tests
- Manual testing
Testing, Continous Integration
- Step 0: run php -l on your files to lint for syntax errors
- Add some "smoke tests" that run relevant parts of your application
- Run them automatically on your CI
- Do not accept them to be failing
Code Coverage
bin/phpunit --coverage-html public/coverage
http://debugging.lo/coverage/index.html
Run only tests/Functional/Controller/HomeControllerTest.php
Code Coverage
- Code coverage report is a tool
- Functional tests tell you if the complete application works
- Unit tests allow you to efficiently test edge cases
- Unit tests allow you to localize bugs
✔ Looking at code coverage helps you identify untested parts of your code.
[p1] Static code analysis
vendor/bin/phpstan analyse --level 1 src/
Fix the reported bug, then try raising the level to find more issues.
[p1] Static code analysis
---- --------------------------------------------
Line src/Importer/Mapper/CustomerMapper.php
---- --------------------------------------------
49 Class App\Importer\Mapper\Customr not found.
---- --------------------------------------------
✔
Static code analyzers push you to write clean code. Sometimes they also find straight-up bugs.
Clean Code
- Testable code
- Single responsibility
- Cyclomatic complexity
Clean Code
if (!$condition) {
// do stuff
} else {
return $default;
}
return $result;
if ($condition) {
return $default;
}
// do stuff
return $result;
Clean Code
foreach ($list as $valueSet) {
foreach ($valueSet as $key => $value) {
if (0 === strpos($value, $searched)) {
$result = $key;
break(2);
}
}
}
Clean Code
function findValue($list, $searched) {
foreach ($list as $valueSet) {
foreach ($valueSet as $key => $value) {
if (0 === strpos($value, $searched)) {
return $key;
}
}
}
}
$result = findValue($list, $searched);
Note: Method calls add overhead, do not exagerate.
[p2] Mess detection
vendor/bin/phpmd src/ text codesize
This is no pretty sight
[p2] Mess detection
- I unwrapped the whole code for you
- Lets not refactor this now
- Avoid such code, refactor it when you encounter it
✔
A mess detector can help you to avoid hard to understand and hard to test code
Mess detection
- Static access to fields or methods
- Else statements
- Code complexity
- Excessive size of class or method
- Too many public methods, too many fields
- Unreachable code
- Development code fragments (var_dump, print_r)
Source: https://twitter.com/_cingraham/status/946462990504681472
Yoda style expressions
if ('404' = errorCode) {
Syntax error instead of accidental value assignment
[p3] Validate - fail early
bin/phpunit => all green
bin/console app:import
Add sanity checks in the CSV loader
[p3] Validate - fail early
The file configuration got swapped, the code tries to load the wrong file.
- Everybody makes stupid errors - including you
- Also notice: Without the typehints in the methods, the problem would have shown up even later.
✔
The earlier you fail, the closer to the source of the problem you are. That simplifies debugging tremendously.
Demo: Symfony OptionsResolver
public function __construct(array $options = [])
{
$resolver = new OptionsResolver();
$resolver->setRequired([
'template',
]);
$resolver->setDefault('choice', 'a');
$resolver->setAllowedValues(
'choice',
['a', 'b', 'c']
);
$this->options = $resolver->resolve($options);
}
[p4] Make it difficult to make mistakes
bin/phpunit
What is wrong?
[p4] isset vs array_key_exists
if (isset($this->customer[$data['Id']])) {
$customer = $this->customers[$data['Id']];
$customer->name = $data['Name'];
return $customer;
}
if (array_key_exists($data['Id'], $this->customers))
✔
isset hides errors, array_key_exists does not use any magic
Make your code understandable
- Provide high level documentation: What does the application do, what are the main components?
- Where appropriate, use code comments to explain the why
- Put effort into good and consistent naming
- Coding standard: Consistent layout.
php-cs-fixer (styleci as a service) or similar.
- Delete unused code.
Make your code understandable (II)
- KISS: Keep it simple. Chose the easiest solution that does the job
- YAGNI: You ain't gonna need it (Program for your use case, not your re-use case)
- Use libraries for solved problems (e.g. Httplug or guzzle over raw curl calls)
https://twitter.com/legacy_coder/status/880138870460997632
Naming things
function createNewUser($username, $dontEscape=false)
function createUser($username, $escape=true)
- Double negation are hard to understand
- "Create" and "New" are redundant
Bug Detection
Monitor
- Exceptions being thrown in your program
- HTTP status codes
- System load (CPU, RAM)
- Throughput (performance decrease because of more requests or code changes?)
Monitoring Tools
- Newrelic: Enterprise - Multi programming languages, multi user, multi organization
Focus: Monitoring and alerting for web servers
- tideways.io: PHP specific - Detect slow requests, produce analysis of requests with time measurements
Focus: Monitoring and alerting for PHP applications
- blackfire.io: PHP specific - Excellent analysis of requests, also on CPU / memory. Compare specific trace over time.
Focus: Analysing specific code paths in PHP applications
Alerting
- Have monitoring notify you when things go bad
- Reduce amount of warnings - either fix them or hide them
Logging
- Analysis what happened after the fact
- Don't forget to configure logrotate
- Log levels: Critical, Error, Warning, Info, Debug
Log Levels
main:
type: fingers_crossed
action_level: error
handler: nested
excluded_404s:
# regex: exclude 404 errors from the logs
- ^/
nested:
type: stream
path: "/var/log/symfony.log"
level: debug
[d1] Demo: Log Channels
Separate some logs into a channel
bin/console app:import --reset -v
bin/console app:import --reset -vv
bin/console app:import --reset -vvv
App\Importer\Mapper\LikeMapper:
arguments:
- '@logger'
tags:
-
name: monolog.logger
channel: like_mapper
debug_like_mapper:
type: console
channels: ["like_mapper"]
level: debug
# console logger checks log level against
# console output verbosity
verbosity_levels:
VERBOSITY_QUIET: ERROR
VERBOSITY_NORMAL: ERROR
VERBOSITY_VERBOSE: DEBUG
VERBOSITY_VERY_VERBOSE: DEBUG
VERBOSITY_DEBUG: DEBUG
Wrap-Up
https://github.com/symfony/symfony/pull/18547
Types of Bugs
- User Interface: Functionality, Communication
- Error Handling: Prevention, Detection, Recovery
- Boundary Errors: Data Structures
- Calculation Errors: Wrong Algorithm, Rounding
- State: Initialization, Re-Running
- Control Flow: Loops, Wrong Conditions
- Data: Misunderstood Values, Out of sync
Types of Bugs (II)
- Race Conditions: Assuming Something Finished, Assume Guaranteed Order
- Testing Problems: Not Covered Functionality & Missing Checks, False Negative (Dependency on other Systems)
Cem Kaner's appendix of "Common Software Errors" (Kaner et al)
Distributed System
- Central logging system
- Transaction ID over all systems
- End to end integration tests
- Defensive at boundaries
Questions / Input / Feedback ?
https://joind.in/talk/1e8f7
Twitter: @dbu