Improve error handling, tests #53
Conversation
There was a problem hiding this comment.
There are few if X is None: statements in code, we should change those to if not X:.
Code + tests look good.
The test steps need a few changes.
- we will have to update the version to 0.1.6 and then run
python3 setup.py sdist - Then in the debian packaging repo, we will have to add a changelog entry to
securedrop-proxy/debian/changelog-busterfor0.1.6. - After building the debian package, we should copy it to the
sd-proxy-buster-template, the test instructions have the wrong template name.
My client can not reach the server after the update to the this branch. I am currently debugging the issue. But, it is not giving much of log.
The client works as expected with this PR. From my side, after the Truth value testing code change, I will approve this one. @rmol great work. 🌈
|
let's do the version bump in another PR (@rmol had asked me about this privately and I said let's do the release separately) since nightlies will build the latest even if we don't cut an official release |
Ah, my suggestion of version bump is for the reviewer for QA, not for the actual PR. |
rmol
force-pushed
the
13-get-graceful
branch
from
e73f384 to
5097c2c
Compare
|
@kushaldas Thanks for the thorough review. I think I've fixed all the callback checks, and addressed the fixture mismatch for |
|
|
||
| def test_cannot_connect(self): | ||
| """ | ||
| Test for "502 Bad Gateway" when the server can't be reached. |
There was a problem hiding this comment.
great tests! note to other reviewers that this provides regression coverage for #13
Try to ensure the proxy always returns a valid JSON response. Add specific handling of upstream error responses to proxy.py, and add tests and fixtures for that. Replace the Proxy._on_done static method with an instance method. In main.py, bail out upon receiving invalid input JSON, instead of printing an error response and then continuing to try to use it, resulting in printing another error response. Have main.py use the callbacks on the proxy as given, instead of always overwriting them with the defaults from callbacks.py. Rework entrypoint.py so that it should always print a JSON response, and add tests for it.
Give test_main.test_input_headers its own fixture. Use "if not callback" instead of "if callback is None".
kushaldas
force-pushed
the
13-get-graceful
branch
from
5097c2c to
e2e9bf3
Compare
Description
Try to ensure the proxy always returns a valid JSON response. Add specific handling of upstream error responses to proxy.py, and add tests and fixtures for that.
Replace the Proxy._on_done static method with an instance method.
In main.py, bail out upon receiving invalid input JSON, instead of printing an error response and then continuing to try to use it, resulting in printing another error response.
Have main.py use the callbacks on the proxy as given, instead of always overwriting them with the defaults from callbacks.py.
Rework entrypoint.py so that it should always print a JSON response, and add tests for it.
Fixes #128.
Testing
securedrop-proxysource distribution:python setup.py sdist.deb:securedrop-debian-packaging, runPKG_PATH=/home/user/src/securedrop-proxy/dist/securedrop-proxy-0.1.6.tar.gz PKG_VERSION=0.1.6 make securedrop-proxy(adjust the path to your working copy of this repo as necessary).debinsd-svs-buster-template:qvm-copy ~/debbuild/packaging/securedrop-proxy_0.1.6+buster_all.deband specifysd-svs-buster-templateas the target VMsd-svs-buster-template, run:cp ~/QubesIncoming/sd-dev/securedrop-proxy_0.1.6+buster_all.deb /tmpsudo apt install /tmp/securedrop-proxy_0.1.6+buster_all.debsd-svsand confirm that the proxy is communicating with the server: submissions and replies should populate with no errors.