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

feat!: Do not treat Buffers as JSON by default #621

Merged
merged 35 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
58c7dc2
feat!: Upgrade to `node-fetch` v3
d-goog Apr 16, 2024
f8570d9
Merge branch 'main' into upgrade-node-fetch
danielbankhead Apr 16, 2024
7fd2a95
refactor: Use native `FormData` in testing
d-goog Apr 17, 2024
0cc7856
Merge branch 'upgrade-node-fetch' of github.com:googleapis/gaxios int…
d-goog Apr 17, 2024
a61cfa5
feat!: Improve spec compliance
d-goog Apr 19, 2024
35a73b1
test(temp): Run 18+
d-goog Apr 19, 2024
e7addeb
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Apr 19, 2024
9816229
feat: Improve types, streamline, and standardize
d-goog May 14, 2024
7315442
feat!: Do Not Treat Buffers as JSON By Default
d-goog May 15, 2024
93bf4a8
Merge branch 'main' of github.com:googleapis/gaxios into upgrade-node…
d-goog May 15, 2024
273e516
Merge branch 'upgrade-node-fetch' of github.com:googleapis/gaxios int…
d-goog May 15, 2024
9164b87
test: Adopt `Headers` type from merge
d-goog May 15, 2024
d01ecde
Merge branch 'upgrade-node-fetch' of github.com:googleapis/gaxios int…
d-goog May 15, 2024
84faea8
feat: Introduce `GaxiosOptionsPrepared` for improved type guarantees
d-goog May 16, 2024
4e6de5f
feat: Ensure `GaxiosOptionsPrepared.url` is always a `URL`
d-goog May 16, 2024
e6c2371
refactor: Clean-up Types & Resolve lint warnings
d-goog May 16, 2024
2089d83
Merge branch 'upgrade-node-fetch' of github.com:googleapis/gaxios int…
d-goog May 16, 2024
98f7009
refactor: streamline `.data` for `Response`
d-goog May 20, 2024
e66c4ba
Merge branch 'upgrade-node-fetch' of github.com:googleapis/gaxios int…
d-goog May 20, 2024
4894e39
docs: Simplify example
d-goog May 22, 2024
a7e2e83
refactor: streamline, no error case
d-goog May 22, 2024
0baab44
Merge branch 'main' of github.com:googleapis/gaxios into do-not-treat…
d-goog Jul 1, 2024
b2ec395
docs: clarification
d-goog Jul 1, 2024
8413f68
feat: Basic `GET` Support for `Request` objects
d-goog Sep 11, 2024
04e264d
test: remove `.only`
d-goog Sep 11, 2024
d163a90
refactor: simplify `httpMethodsToRetry`
d-goog Sep 11, 2024
705fad3
chore: update `nock`
d-goog Sep 11, 2024
9e52755
test: cleanup
d-goog Sep 11, 2024
e5baa94
Merge branch 'upgrade-node-fetch' into do-not-treat-buffer-as-json
d-goog Sep 11, 2024
0eb8882
Merge branch 'main' of github.com:googleapis/gaxios into do-not-treat…
d-goog Oct 29, 2024
948a7d4
test: add test for asserting no default `content-type`
d-goog Oct 30, 2024
f91ae28
Merge branch 'main' of github.com:googleapis/gaxios into do-not-treat…
d-goog Oct 30, 2024
bbd56f2
docs: Add detail for `code` property
d-goog Oct 30, 2024
df3565f
docs: uniform
d-goog Oct 30, 2024
9df83c3
Merge branch 'main' of github.com:googleapis/gaxios into do-not-treat…
d-goog Oct 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,22 @@ export const GAXIOS_ERROR_SYMBOL = Symbol.for(`${pkg.name}-gaxios-error`);
/* eslint-disable-next-line @typescript-eslint/no-explicit-any */
export class GaxiosError<T = any> extends Error {
/**
* An Error code.
* See {@link https://nodejs.org/api/errors.html#errorcode error.code}
* An error code.
* Can be a system error code, DOMException error name, or any error's 'code' property where it is a `string`.
*
* @see {@link https://nodejs.org/api/errors.html#errorcode error.code}
* @see {@link https://developer.mozilla.org/en-US/docs/Web/API/DOMException#error_names DOMException#error_names}
*
* @example
* 'ECONNRESET'
*
* @example
* 'TimeoutError'
*/
code?: string;
/**
* An HTTP Status code.
* See {@link https://developer.mozilla.org/en-US/docs/Web/API/Response/status Response: status property}
* @see {@link https://developer.mozilla.org/en-US/docs/Web/API/Response/status Response#status}
*
* @example
* 500
Expand Down
11 changes: 0 additions & 11 deletions src/gaxios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,17 +395,6 @@ export class Gaxios {
) as {} as ReadableStream;
} else if (shouldDirectlyPassData) {
opts.body = opts.data as BodyInit;

/**
* Used for backwards-compatibility.
*
* @deprecated we shouldn't infer Buffers as JSON
*/
if ('Buffer' in globalThis && Buffer.isBuffer(opts.data)) {
if (!preparedHeaders.has('content-type')) {
preparedHeaders.set('content-type', 'application/json');
}
}
} else if (typeof opts.data === 'object') {
if (
preparedHeaders.get('Content-Type') ===
Expand Down
22 changes: 10 additions & 12 deletions test/test.getch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1158,20 +1158,18 @@ describe('🍂 defaults & instances', () => {
assert.deepStrictEqual(res.data, {});
});

it('should set content-type to application/json by default, for buffer', async () => {
Copy link
Contributor

@sofisl sofisl Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we write a test that asserts the behavior for what happens if a buffer does get sent in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it’s mostly a partial revert of:

but that can be added.

const pkg = fs.readFileSync('./package.json');
const pkgJson = JSON.parse(pkg.toString('utf8'));
it('should not set a default content-type for buffers', async () => {
const jsonLike = '{}';
const data = Buffer.from(jsonLike);
const scope = nock(url)
.matchHeader('content-type', 'application/json')
.post('/', pkgJson)
.reply(200, {});
const res = await request({
url,
method: 'POST',
data: pkg,
});
// no content type should be present
.matchHeader('content-type', v => v === undefined)
.post('/', jsonLike)
.reply(204);

const res = await request({url, method: 'POST', data});
scope.done();
assert.deepStrictEqual(res.data, {});
assert.equal(res.status, 204);
});

describe('mtls', () => {
Expand Down