1 |
Hi, my name is Calvin Shin, and I am a former FB Open Academy student who was |
2 |
assigned with fixing a bug, and I would like to explain how I fixed this bug. |
3 |
Let's get to it! |
4 |
|
5 |
The initial issue at hand was that as an admin of Review Board, you could not |
6 |
add a private Github or Kiln repository. It displayed a message that read |
7 |
" 'URLError' object has no attribute 'read()'." Once I tackled this, I |
8 |
changed the error message so that it could properly convey why the error was occurring. |
9 |
|
10 |
Now, for a step-by-step process on how I tackled this HostingService bug-fix. |
11 |
First, the error message that popped up when I reproduced this bug was a good |
12 |
indicator of where I should look in the code to start. Keep in mind the scale |
13 |
of the Review Board codebase in comparison to that of your class assignments |
14 |
and projects. So the error message really saved me a lot of time. Debugging |
15 |
can really get you lost for hours with a codebase like Review Board. |
16 |
Anyways, proceeding with this "hint," I did a quick find in Pycharm, and |
17 |
along with the help of a mentor at my school's Code Sprint, I found the |
18 |
section of the code where it could have been producing that message to |
19 |
appear. This lead me to a part of the code in github.py, into a function |
20 |
called 'authorize'. I looked around in this function and saw the line |
21 |
"data = e.read()". At this point, I saw that the line was in a clause to |
22 |
handle a HTTPError and a URLError and thought that one possibility (deducing |
23 |
from the error message) could be that e was a URLError and that the line |
24 |
"data = e.read()" was causing this message because a URLError object doesn't |
25 |
have the ability to call the read() function. Confirming this with a mentor, |
26 |
I fixed this by making a simple change. I put the exact same line inside a |
27 |
try block that was previously right underneath it. My reasoning for this was |
28 |
that it was failing because data contains an invalid statement (URLError |
29 |
doesn't have attribute read) and so putting it in the try block would attempt |
30 |
to execute that line, but if it didn't work, it would just go ahead to the |
31 |
except block, which was right after the try block. My mentor said to make |
32 |
the same changes where there were other areas of the code that had similar |
33 |
lines, and so I made the same changes in another function in github.py called |
34 |
'_check_api_error', and a function in kiln.py called |
35 |
'_check_api_error_from_exception'. I tried reproducing the bug with the same |
36 |
steps to see if I had fixed the bug, but this time I encountered another |
37 |
error. This one gave another error message reading 'URLError object has no |
38 |
attribute code'. This time, I traced the code back to '_check_api_error', |
39 |
because that was the only place out of the fixes in github.py that I made |
40 |
that contained any text of "code." I saw that the variable e, which was |
41 |
assigned URLError, was calling something that it couldn't; this time, |
42 |
something called code. Only HTTPError, a subclass of URLError, has attribute |
43 |
"code," so I fixed this problem by handling these two errors separately |
44 |
(because both HTTPError and URLError could be assigned as e). If you take a |
45 |
look at '_check_api_error' now, you can see that HTTPError and URLError are |
46 |
handled separately. This is the last set of if-else statements in the |
47 |
function. I changed it so that the HostingServiceError, which previously had |
48 |
the variable http_code=e.code as its second parameter, does not have a second |
49 |
parameter and instead passes in the default parameter set for http_code, |
50 |
which is assigned None. This finally allowed Review Board to display the |
51 |
correct reason for the inability to add a Github repository, but this message |
52 |
showed "<urlopen error [Errno 61] Connection refused>". Because this error |
53 |
message has no significance whatsoever to a user of ReviewBoard, I changed |
54 |
this to read "Could not connect to the server "<domain name>". If the server |
55 |
is behind a proxy, make sure the proxy settings are correct." This message |
56 |
essentially says tells the user that the reason that he or she cannot add a |
57 |
repository is because Python isn't properly configured to a proxy server. |
58 |
|
59 |
Cool, so it seems like the bug is fixed, but this is only the halfway mark in |
60 |
a sense. Now that we have what we think is working, we need to verify with |
61 |
unit tests. Unit tests ensure that whatever behavior we are expecting works |
62 |
properly and accordingly and does not have any changes. This was the part I |
63 |
had the most complications with, mainly because I was required to trace a lot |
64 |
of code and "deep-dive" into code I wasn't working with or familiar with. On |
65 |
top of all this, I only had minimal experience writing my own unit tests for |
66 |
working code. |
67 |
|
68 |
|
69 |
Let's take a quick detour to give a sense of how to trace code. In Pycharm, |
70 |
you can use a shortcut by pressing on the cmd (Macs only) key and clicking on |
71 |
a variable or a function or a class. This will directly lead you to that, in |
72 |
whichever file it is in. For instance, let's take a look at a function called |
73 |
'get_file_exists', in kiln.py, which is an example of an API call (explained |
74 |
in the next paragraph). Starting with this function, you can see that it |
75 |
returns True, if it successfully calls another function called 'get_file'. |
76 |
Using the keyboard shortcut will directly go to the function in whichever |
77 |
file it is contained in. This shortcut leads you to the 'get_file' function, |
78 |
and observing it, you can see that it calls another function called |
79 |
'get_raw_file', which in turn calls 'api_get' on a url. Finally, this calls |
80 |
'http_get' on that url, which performs a GET request on that url. This is |
81 |
assigned to a variable called data and returned. |
82 |
|
83 |
Going back to my own unit test, I started off with writing an individual unit |
84 |
test for each individual fix. So for instance, I made two fixes in github.py |
85 |
and one fix in kiln.py, even though they were contributing to just one bug. |
86 |
I wrote 3 individual unit tests, two to test using a Github repo and another |
87 |
one to test using a Kiln repo. I will explain my writing of the unit tests |
88 |
using the github unit test that tests '_check_api_error'. I first started |
89 |
off with writing the unit test function signature, which is simply |
90 |
'def test_check_url_error (self)'. All unit tests in Python begin with test |
91 |
and do not have other parameters. Secondly, I used the concept of a "spy." |
92 |
A spy essentially intercepts and records calls to functions (more about |
93 |
function spies here: https://github.com/beanbaginc/kgb). In this case, I |
94 |
"spy on" the http_get function and faked the original function so that it |
95 |
will raise a URLError, for the purposes of testing our code. After this, I |
96 |
retrieve the proper hosting service and finally call assertRaisesMessage, |
97 |
passing in the error message that I fixed, so that the test can extract a |
98 |
known result. In the call to assertRaisesMessage, I make an API call by |
99 |
calling '_test_check_repository' (which calls '_check_api_error'), so that |
100 |
the unit test can actually test something that the HostingService would do. |
101 |
In this case, it is to check a repository; this function makes a proper API |
102 |
call because it reaches 'api_get', which calls 'http_get', the function that |
103 |
is spied on to raise a URLError. |
104 |
|
105 |
|
106 |
Ultimately, this bug that I thought I had fixed at the code sprint, or at |
107 |
least by the end of my first week, resulted in me working on this as |
108 |
essentially another project, along with the project I had chosen to work on |
109 |
throughout the term. I honestly was quite annoyed when everybody else |
110 |
finished their bug fixes their first week and spent the majority of their |
111 |
time on their individual projects. However, from this one bug, I learned how |
112 |
to trace and follow code, from one function to another, in a huge project |
113 |
with a lot of files and lines of code, an invaluable skill for any developer. |
114 |
I learned how to write unit tests, also an essential skill in industry. |
115 |
Finally, I gained insight into debugging, which is, in and of itself, another |
116 |
aspect of software engineering. Thank you for reading this, and I hope you |
117 |
are now equipped to debug your bugs! |