refactor: improve numtracker error messages#1538
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1538 +/- ##
=======================================
Coverage 95.35% 95.35%
=======================================
Files 43 43
Lines 3205 3209 +4
=======================================
+ Hits 3056 3060 +4
Misses 149 149 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
tpoliaw
left a comment
tpoliaw
left a comment
There was a problem hiding this comment.
Looks good. None of the comments are blockers
| for error in json["errors"]: | ||
| code = (error.get("extensions") or {}).get("code") | ||
|
|
||
| if code == "AUTH_FAILED": |
There was a problem hiding this comment.
Do we want to improve the error message for the other cases while we're here? Could be a good use for match - something like
match code:
case "AUTH_FAILED":
raise ...
case "AUTH_MISSING":
raise ...
case "AUTH_SERVER_ERROR":
raise ...and then the fall-through case could be the existing Numtracker error: ... we were previously raising.
There was a problem hiding this comment.
Yuss that's a good idea thanks
| "data": None, | ||
| "errors": [ | ||
| { | ||
| "message": "No configuration available for instrument p46", |
There was a problem hiding this comment.
really minor but having the message match the error might prevent confusion later on
2 participants
Fixes #1507
Summary of changes: