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

Correctly parse response bodies as JSON where the Content-Type is application/scim+json #731

Merged
merged 5 commits into from
Jan 16, 2025

Conversation

timrogers
Copy link
Contributor

@timrogers timrogers commented Jan 16, 2025

GitHub has APIs that return application/scim+json response bodies. Currently, these responses are not parsed as JSON, and instead, an ArrayBuffer is returned in response.data.

This adds handling for application/scim+json so they are parsed as JSON as normal.

Minimal reproduction

import { Octokit } from "@octokit/rest";
import { enterpriseCloud } from "@octokit/plugin-enterprise-cloud";

const MyOctokit = Octokit.plugin(enterpriseCloud);
const myOctokit = new MyOctokit({
  auth: "ghp_xxxxxxxx",
});

const resp = await myOctokit.request('GET /scim/v2/enterprises/{enterprise}/Groups', {
  enterprise: 'fabrikam'
});
console.log(JSON.stringify(resp.data));

Detailed changes

  • Modify getResponseData function in src/fetch-wrapper.ts to include a check for application/scim+json content type.
  • Refactor content type check into a new isJSONResponse function.
  • Add a test in test/request.test.ts to verify that response bodies with application/scim+json content type are correctly parsed as JSON.

For more details, open the Copilot Workspace session.

…plication/scim+json`

GitHub has APIs that return `application/scim+json` response bodies. Currently, these responses are not parsed as JSON, and instead, an `ArrayBuffer` is returned in `response.data`.

This adds handling for `application/scim+json` so they are parsed as JSON as normal.

## Minimal reproduction

```js
import { Octokit } from "@octokit/rest";
import { enterpriseCloud } from "@octokit/plugin-enterprise-cloud";

const MyOctokit = Octokit.plugin(enterpriseCloud);
const myOctokit = new MyOctokit({
  auth: "ghp_xxxxxxxx",
});

const resp = await myOctokit.request('GET /scim/v2/enterprises/{enterprise}/Groups', {
  enterprise: 'fabrikam'
});
console.log(JSON.stringify(resp.data));

## Detailed changes

* Modify `getResponseData` function in `src/fetch-wrapper.ts` to include a check for `application/scim+json` content type.
* Refactor content type check into a new `isJSONResponse` function.
* Add a test in `test/request.test.ts` to verify that response bodies with `application/scim+json` content type are correctly parsed as JSON.

---

For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/octokit/request.js?shareId=XXXX-XXXX-XXXX-XXXX).
Copy link

👋 Hi! Thank you for this contribution! Just to let you know, our GitHub SDK team does a round of issue and PR reviews twice a week, every Monday and Friday! We have a process in place for prioritizing and responding to your input. Because you are a part of this community please feel free to comment, add to, or pick up any issues/PRs that are labeled with Status: Up for grabs. You & others like you are the reason all of this works! So thank you & happy coding! 🚀

src/fetch-wrapper.ts Outdated Show resolved Hide resolved
@wolfy1339 wolfy1339 merged commit 00bf316 into octokit:main Jan 16, 2025
7 checks passed
Copy link

🎉 This PR is included in version 9.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants