Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changes to x-task-id parsing logic for responses without a json body #27

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

sbarbett
Copy link
Contributor

@sbarbett sbarbett commented Nov 8, 2024

There's still a problem with how the connection logic handles adding the task ID.

Creating zone snapshots:   0%|                                                                                                                                                          | 0/315 [00:00<?, ?it/s]{'Access-Control-Allow-Headers': 'Origin, X-Requested-With, Content-Type, Accept, Authorization', 'Access-Control-Allow-Methods': 'POST, GET, OPTIONS, DELETE, PUT, PATCH', 'Access-Control-Allow-Origin': '*', 'Access-Control-Max-Age': '3600', 'Content-Security-Policy': "default-src 'self' https:; font-src 'self' https: data:; img-src 'self' https: data:; object-src 'none'; script-src https:; style-src 'self' https: 'unsafe-inline'", 'Date': 'Fri, 08 Nov 2024 14:06:38 GMT', 'Referrer-Policy': 'no-referrer', 'Strict-Transport-Security': 'max-age=31536000; includeSubDomains; preload', 'Vary': 'Origin, Access-Control-Request-Method, Access-Control-Request-Headers', 'X-Content-Type-Options': 'nosniff', 'X-Download-Options': 'noopen', 'X-Frame-Options': 'DENY', 'X-Permitted-Cross-Domain-Policies': 'none', 'X-Task-Id': '14b14a0e-42c3-412d-840a-126b66b2bbe7', 'X-Xss-Protection': '1 ; mode=block', 'Content-Length': '0', 'Connection': 'keep-alive'}
202
{}
Creating zone snapshots:   0%|                                                                                                                                                          | 0/315 [00:00<?, ?it/s]
Traceback (most recent call last):
  File "/Users/sbarbett/projects/udns_snapshot/./src/snapshot.py", line 270, in <module>
    main(args.username, args.password, args.token, args.refresh_token, args.restore, args.log_file, args.download, args.zones_file)
  File "/Users/sbarbett/projects/udns_snapshot/./src/snapshot.py", line 234, in main
    task = create_snapshot(client, zone)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/sbarbett/projects/udns_snapshot/./src/snapshot.py", line 116, in create_snapshot
    task = response["task_id"]
           ~~~~~~~~^^^^^^^^^^^
KeyError: 'task_id'

In the output above I'd like to highlight a couple things.

  1. In the response header, there's an X-Task-Id and the status code is 202 - This indicates that the request was successful.
  2. The {} indicates that the response from the client is empty, so the task_id, despite being present, was not added to the dict.
  3. My code fails because it was expecting a task_id

Looking at the logic here, it's because of how the parsing of the JSON body and try...catch statement are structured.

json_body = {}
try:
  json_body = response.json()

  # if this is a background task, add the task id (or location) to the body
  if response.status_code == requests.codes.ACCEPTED:
    if 'x-task-id' in response.headers:
      json_body.update({"task_id": response.headers['x-task-id']})
    if 'location' in response.headers:
      json_body.update({"location": response.headers['location']})

except requests.exceptions.JSONDecodeError:
  json_body = {}

The response from this API contains no body, so json_body = response.json() is going to fail, but the if statement is nested inside this try...catch. Let's see what happens if I print the exception message.

except requests.exceptions.JSONDecodeError as e:
  print(e)
  json_body = {}

I get the following message.

Expecting value: line 1 column 1 (char 0)

This supports my suspicion, that it can't parse the empty value and is therefore raising an exception and not making it to the conditional statements.

I think the parsing of the hypothetical json body using a try...catch block and conditional statements should happen independently of each other to avoid this issue.

for responses without a json body
@stevedejong stevedejong merged commit c3d6a16 into ultradns:master Dec 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants