Skip to content

Conversation

@Eijebong
Copy link
Contributor

…fact

The previous code was only checking if the returned response from the taskcluster API was a dict to verify if it had been parsed or not. That works well until the JSON file we're getting contains something like a list in which case we skip the first condition, end up with a parsed response in if path.endswith(".json") and boom AttributeError: 'list' object has no attribute 'json'.

Since we already rely on the response type being requests.Request, I changed the logic to be:

  • If we receive a dict, it might be either the parsed artifact content, or taskcluster giving us {"response": requests.Response} because it was unable to parse it.
  • In the case where it's taskcluster's (detected by checking if "response" is in the dict and if it has the right type), peel it off, response is now the Response object and we can read from it.
  • At this point we have 2 choices again, either response is a request.Response that we have to read, or anything else. If it is anything else then it's already been parsed by taskcluster so we just return it as is.
  • Continue with the previous logic, try to parse the file based on the extension. I left the JSON case in there even though we should not be hitting it because it will raise on invalid JSON, which was the case previously too. Taskcluster masks JSON failures by returning the response object instead.

@Eijebong Eijebong requested a review from a team as a code owner October 30, 2025 10:12
@Eijebong Eijebong requested a review from bhearsum October 30, 2025 10:12
…fact

The previous code was only checking if the returned response from the
taskcluster API was a dict to verify if it had been parsed or not. That
works well until the JSON file we're getting contains something like a
list in which case we skip the first condition, end up with a parsed
response in `if path.endswith(".json")` and boom `AttributeError: 'list'
object has no attribute 'json'`.

Since we already rely on the response type being `requests.Request`, I
changed the logic to be:

- If we receive a dict, it might be either the parsed artifact content,
  or taskcluster giving us `{"response": requests.Response}` because it
  was unable to parse it.
- In the case where it's taskcluster's (detected by checking if
  "response" is in the dict and if it has the right type), peel it off,
  response is now the `Response` object and we can read from it.
- At this point we have 2 choices again, either response is a
  `request.Response` that we have to read, or anything else. If it is
  anything else then it's already been parsed by taskcluster so we just
  return it as is.
- Continue with the previous logic, try to parse the file based on the
  extension. I left the JSON case in there even though we should not be
  hitting it because it will raise on invalid JSON, which was the case
  previously too. Taskcluster masks JSON failures by returning the
  response object instead.
@Eijebong Eijebong merged commit c884607 into taskcluster:main Oct 30, 2025
18 checks passed
abhishekmadan30 pushed a commit to abhishekmadan30/taskgraph that referenced this pull request Dec 24, 2025
…fact (taskcluster#846)

The previous code was only checking if the returned response from the
taskcluster API was a dict to verify if it had been parsed or not. That
works well until the JSON file we're getting contains something like a
list in which case we skip the first condition, end up with a parsed
response in `if path.endswith(".json")` and boom `AttributeError: 'list'
object has no attribute 'json'`.

Since we already rely on the response type being `requests.Request`, I
changed the logic to be:

- If we receive a dict, it might be either the parsed artifact content,
  or taskcluster giving us `{"response": requests.Response}` because it
  was unable to parse it.
- In the case where it's taskcluster's (detected by checking if
  "response" is in the dict and if it has the right type), peel it off,
  response is now the `Response` object and we can read from it.
- At this point we have 2 choices again, either response is a
  `request.Response` that we have to read, or anything else. If it is
  anything else then it's already been parsed by taskcluster so we just
  return it as is.
- Continue with the previous logic, try to parse the file based on the
  extension. I left the JSON case in there even though we should not be
  hitting it because it will raise on invalid JSON, which was the case
  previously too. Taskcluster masks JSON failures by returning the
  response object instead.
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