fix: improve error handling when request receives non-JSON response#1477
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1477 +/- ##
==========================================
+ Coverage 95.21% 95.22% +0.01%
==========================================
Files 43 43
Lines 3132 3144 +12
==========================================
+ Hits 2982 2994 +12
Misses 150 150 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
566bd02 to
ba9a28b
Compare
tpoliaw
left a comment
There was a problem hiding this comment.
I'm not sure the _exception method is the best place for this check to be, for instance running a plan uses the alternative _create_task_exceptions method and bypasses this check
$ # with config pointing to oauth-proxy
$ uv run blueapi -c path/to/config.yaml controller run --bg -i cm12345-3 sleep '{"time": 3}'
Error: task could not run: Expecting value: line 1 column 1 (char 0)
We're also parsing the json multiple times on the happy path when everything is working which isn't ideal.
Would it work to add a helper method to wrap wherever we're calling .json()?
98a4f7c to
55f732f
Compare
tpoliaw
left a comment
There was a problem hiding this comment.
This is much better, thanks. A couple of more minor comments if you want to look at them but this is a big improvement over the stacktrace it gave before.
improve error handling when request receives non-JSON response.
Closes 1472