WEBVTT

NOTE Created by CaptionSync from Automatic Sync Technologies www.automaticsync.com

00:00:01.126 --> 00:00:03.366 align:middle
We just reviewed our first pull request!

00:00:04.116 --> 00:00:07.346 align:middle
So let's see what other trouble we can get into!

00:00:08.386 --> 00:00:11.996 align:middle
One of the most important thing
you can do is to triage issues.

00:00:13.016 --> 00:00:17.346 align:middle
If an issue is for a feature request, then
it's probably a discussion about whether

00:00:17.346 --> 00:00:20.076 align:middle
or not it's a good idea and
the best implementation.

00:00:20.956 --> 00:00:23.176 align:middle
Helping those discussions is great.

00:00:23.616 --> 00:00:26.456 align:middle
But... click on the "bug" label.

00:00:27.646 --> 00:00:30.376 align:middle
These are where you can really help.

00:00:31.396 --> 00:00:33.866 align:middle
Symfony is pretty stable & complex.

00:00:34.486 --> 00:00:38.966 align:middle
So, if there is a bug, it's
usually pretty complicated

00:00:39.226 --> 00:00:42.246 align:middle
or involves some edge-case situation.

00:00:43.256 --> 00:00:46.346 align:middle
These can take a lot of time
to understand and replicate.

00:00:47.016 --> 00:00:51.126 align:middle
The core team really needs help from
the community to verify the bug,

00:00:51.126 --> 00:00:57.756 align:middle
ask for more information from the user
and, ultimately, to create a "reproducer":

00:00:58.236 --> 00:01:01.296 align:middle
a tiny app that clearly shows
that bug in action.

00:01:02.386 --> 00:01:07.156 align:middle
Let's triage an issue I found
a few days ago: #27901.

00:01:08.476 --> 00:01:14.546 align:middle
Ok, the user says that he got an error when
trying to serialize a Doctrine QueryBuilder

00:01:14.726 --> 00:01:20.716 align:middle
with the web profiler: something about not
being able to serialize a PDO instance.

00:01:22.016 --> 00:01:26.376 align:middle
The web profiler works by collecting a
bunch of information about the request

00:01:26.666 --> 00:01:29.236 align:middle
and serializing it to a cache file.

00:01:30.196 --> 00:01:32.626 align:middle
It looks like something failed
during that process.

00:01:33.616 --> 00:01:38.966 align:middle
This is actually a pretty nice bug report
because he lists the steps to reproduce:

00:01:39.976 --> 00:01:43.446 align:middle
install the web profiler
and then call execUpdate

00:01:43.706 --> 00:01:49.026 align:middle
on Doctrine's lower-level Connection
object, passing it a QueryBuilder.

00:01:50.046 --> 00:01:52.466 align:middle
He even suggests a solution!

00:01:53.946 --> 00:01:56.236 align:middle
Ok, so, how can we help?

00:01:57.536 --> 00:02:01.326 align:middle
First: see if we can reproduce
& understand the issue.

00:02:02.546 --> 00:02:04.706 align:middle
Let's create another small project for this.

00:02:05.716 --> 00:02:11.986 align:middle
Notice that this is an issue that's reported
on the stable version of Symfony: 4.1.

00:02:12.866 --> 00:02:20.156 align:middle
So, we should create a new app based on that
same version - not dev-master like before.

00:02:22.106 --> 00:02:28.106 align:middle
Press Ctrl+C to stop the server, move back
up to the top contributing directory and run:

00:02:28.516 --> 00:02:35.396 align:middle
composer create-project symfony/skeleton
triage_issue_27901

00:02:36.776 --> 00:02:42.366 align:middle
Because we're not specifying a version, it
will use the current stable version: 4.1.

00:02:44.606 --> 00:02:47.056 align:middle
When this finishes, move into the directory.

00:02:49.236 --> 00:02:52.786 align:middle
To replicate this bug, we will at least need

00:02:52.786 --> 00:02:56.606 align:middle
to install the stuff he's using:
Doctrine and the web profiler.

00:02:57.776 --> 00:03:00.816 align:middle
Back at the terminal, just
install Doctrine for now

00:03:00.986 --> 00:03:09.886 align:middle
so we can write some code:
composer require orm And...

00:03:10.316 --> 00:03:14.766 align:middle
done! Close the old directory
and open this new one.

00:03:16.906 --> 00:03:25.266 align:middle
Then, go straight to create a new controller
class: how about Issue27901Controller.

00:03:29.176 --> 00:03:31.106 align:middle
Give it a public function test().

00:03:32.676 --> 00:03:34.706 align:middle
Ok: check back on the issue.

00:03:36.076 --> 00:03:42.526 align:middle
He's using the Doctrine Connection object -
a lower-level object I don't use too often.

00:03:43.906 --> 00:03:46.406 align:middle
To see out how to get it,
find your terminal and run:

00:03:46.816 --> 00:03:51.376 align:middle
php bin/console debug:autowiring and scroll up.

00:03:54.106 --> 00:03:59.936 align:middle
Yep! It looks like we can type-hint a
Connection class to get the service we need.

00:04:00.976 --> 00:04:04.856 align:middle
Do that: Connection $connection.

00:04:07.306 --> 00:04:12.326 align:middle
Next, he calls execUpdate() and
passes it a QueryBuilder argument.

00:04:13.296 --> 00:04:16.486 align:middle
You may already be familiar with
the QueryBuilder from Doctrine.

00:04:16.846 --> 00:04:21.936 align:middle
Well, in this case, because we're
using the lower-level Connection class

00:04:22.036 --> 00:04:29.526 align:middle
from the Doctrine dbal library, the QueryBuilder
is also a lower-level class from that library.

00:04:30.616 --> 00:04:35.246 align:middle
These are the types of little details
that can make triaging a bug tough!

00:04:35.246 --> 00:04:39.556 align:middle
But, it's also part of the
fun: you'll need to really dig

00:04:39.556 --> 00:04:41.736 align:middle
into the code to find out what's going on.

00:04:42.806 --> 00:04:46.696 align:middle
Create the QueryBuilder with
$connection->createQueryBuilder().

00:04:47.406 --> 00:04:51.466 align:middle
I won't even do anything with it
yet: we're still investigating.

00:04:52.976 --> 00:04:55.576 align:middle
Next, he calls execUpdate().

00:04:58.406 --> 00:05:00.586 align:middle
Oh, but that doesn't exist!

00:05:01.246 --> 00:05:05.296 align:middle
I bet he meant execteUpdate() - pass that $qb.

00:05:05.296 --> 00:05:13.006 align:middle
Great! At this point, I would normally install
the web profiler, create some database entities

00:05:13.206 --> 00:05:17.386 align:middle
and use a real query in the controller
to see if we can replicate the error.

00:05:18.026 --> 00:05:24.246 align:middle
But, before we do that, I noticed
something: the first argument looks

00:05:24.376 --> 00:05:26.926 align:middle
like it's supposed to be a string!

00:05:27.746 --> 00:05:35.466 align:middle
Hold Command or Ctrl and click to
open the executeUpdate() method.

00:05:35.466 --> 00:05:37.826 align:middle
Yep! The first argument should be a string!

00:05:38.586 --> 00:05:42.346 align:middle
But, the user is passing a QueryBuilder object!

00:05:43.386 --> 00:05:46.036 align:middle
In other words, I don't think this is a bug!

00:05:47.016 --> 00:05:51.276 align:middle
The only reason the user's
query actually works is that,

00:05:51.616 --> 00:05:56.986 align:middle
if you open the QueryBuilder class,
it has a __toString() method.

00:05:57.986 --> 00:06:03.836 align:middle
Doctrine is probably accidentally converting
this object to a string and using that SQL.

00:06:04.856 --> 00:06:11.976 align:middle
This is why his possible solution is to,
inside a related class, convert the sql -

00:06:12.286 --> 00:06:15.856 align:middle
which is a QueryBuilder in
his case - into a string.

00:06:17.046 --> 00:06:21.426 align:middle
That would fix things, but I
don't think this is really a bug.

00:06:22.016 --> 00:06:25.726 align:middle
But even still, it's awesome
that the user opened this issue.

00:06:26.536 --> 00:06:31.226 align:middle
In a lot of cases, even if there
is no bug, we can use the mistake

00:06:31.306 --> 00:06:34.866 align:middle
to improve things, like with
better error messages.

00:06:35.786 --> 00:06:37.136 align:middle
So, let's reply!

00:06:37.516 --> 00:06:44.336 align:middle
And give as many specific reasons why we
think this might not be a bug: in this case,

00:06:44.496 --> 00:06:46.956 align:middle
that the first argument expects a string.

00:06:48.256 --> 00:06:51.106 align:middle
As extra credit, I'll link to this exact code.

00:06:53.006 --> 00:06:55.746 align:middle
Go to the doctrine/dbal repository.

00:06:56.706 --> 00:07:00.846 align:middle
Then, press the letter "t"
to open this search screen.

00:07:01.766 --> 00:07:03.856 align:middle
I live by this shortcut.

00:07:05.046 --> 00:07:07.446 align:middle
Look for Connection.php and open it.

00:07:10.546 --> 00:07:14.876 align:middle
Search for executeUpdate() and...

00:07:14.996 --> 00:07:20.186 align:middle
click to select that line: this
updates the URL to point here.

00:07:21.346 --> 00:07:25.966 align:middle
Then - here's another trick - press the "y" key.

00:07:27.046 --> 00:07:31.556 align:middle
This changes the URL from
master to the actual commit sha.

00:07:32.476 --> 00:07:39.886 align:middle
This helps make sure that this link - to line
1068 - will forever point to the line we want -

00:07:40.986 --> 00:07:45.166 align:middle
even if someone makes changes on the
master branch and moves this line.

00:07:47.206 --> 00:07:49.356 align:middle
I'll paste the link and add a few more details.

00:07:50.246 --> 00:07:54.546 align:middle
I really try to be as friendly
as possible: this is our chance

00:07:54.546 --> 00:07:57.486 align:middle
to help make Symfony a warm
& welcoming community.

00:07:58.146 --> 00:08:07.186 align:middle
Even if this is not a valid issue, it's great
the user took their time to help report it.

00:08:07.186 --> 00:08:08.336 align:middle
And... boom!

00:08:08.336 --> 00:08:10.696 align:middle
You probably won't have the
power to close the issue,

00:08:11.156 --> 00:08:14.196 align:middle
but this should make it easy
for someone else to do that.

00:08:14.986 --> 00:08:16.566 align:middle
Achievement unlocked!

00:08:17.916 --> 00:08:20.506 align:middle
This bug turned out to not be a bug.

00:08:21.226 --> 00:08:25.076 align:middle
So, let's hunt for a bug that really is a bug.

00:08:25.496 --> 00:08:29.956 align:middle
And learn how to create and
share a "reproducer" project...

00:08:30.456 --> 00:08:34.786 align:middle
which is seriously almost as
valuable as actually fixing the bug.