From 65a4459e5d02cf74bc59a71652e2c79f00f61f3f Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 2 Jul 2021 22:05:30 +0700 Subject: [PATCH 01/11] failing tests for 1351 --- .../basics/src/routes/endpoint-output/_tests.js | 13 +++++++++++++ .../apps/basics/src/routes/endpoint-output/empty.js | 7 +++++++ 2 files changed, 20 insertions(+) create mode 100644 packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js create mode 100644 packages/kit/test/apps/basics/src/routes/endpoint-output/empty.js diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js new file mode 100644 index 000000000000..1d7e23810fff --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js @@ -0,0 +1,13 @@ +import * as assert from 'uvu/assert'; + +/** @type {import('test').TestMaker} */ +export default function (test) { + test.only('not ok on void endpoint', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/empty', { method: 'DELETE' }); + assert.equal(res.ok, false); + }); + test.only('200 status on empty endpoint', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/empty'); + assert.equal(res.status, 200); + }); +} diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/empty.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/empty.js new file mode 100644 index 000000000000..78ba9f6fb252 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/empty.js @@ -0,0 +1,7 @@ +/** @type {import('@sveltejs/kit').RequestHandler} */ +export function get() { + return {}; +} + +/** @type {import('@sveltejs/kit').RequestHandler} */ +export function del() {} From 2bbf4718d6dc4bc51ba85ab748108873a22482d0 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 2 Jul 2021 22:29:38 +0700 Subject: [PATCH 02/11] fix: add default value to response body --- packages/kit/src/runtime/server/endpoint.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 155e81c182f0..21919c600d41 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -33,7 +33,7 @@ export default async function render_route(request, route) { ); } - let { status = 200, body, headers = {} } = response; + let { status = 200, body = '', headers = {} } = response; headers = lowercase_keys(headers); const type = headers['content-type']; From e8c5180e57a03263157bfc089f63cdef7dda431e Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 2 Jul 2021 22:36:14 +0700 Subject: [PATCH 03/11] test: set cookie without body --- .../test/apps/basics/src/routes/endpoint-output/_tests.js | 6 ++++++ .../apps/basics/src/routes/endpoint-output/headers.js | 8 ++++++++ 2 files changed, 14 insertions(+) create mode 100644 packages/kit/test/apps/basics/src/routes/endpoint-output/headers.js diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js index 1d7e23810fff..bdf5cf7ab34b 100644 --- a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js @@ -10,4 +10,10 @@ export default function (test) { const res = await fetch('/endpoint-output/empty'); assert.equal(res.status, 200); }); + + test.only('set-cookie without body', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/headers'); + assert.equal(res.status, 200); + assert.equal(res.headers.has('set-cookie'), true); + }); } diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/headers.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/headers.js new file mode 100644 index 000000000000..481f0f882e4d --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/headers.js @@ -0,0 +1,8 @@ +/** @type {import('@sveltejs/kit').RequestHandler} */ +export function get() { + return { + headers: { + 'Set-Cookie': 'foo=bar' + } + }; +} From 130210be9180a13cef62173cabc68cd6aba8f531 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 2 Jul 2021 22:41:27 +0700 Subject: [PATCH 04/11] prefer empty object so .json won't fail --- packages/kit/src/runtime/server/endpoint.js | 2 +- .../kit/test/apps/basics/src/routes/endpoint-output/_tests.js | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 21919c600d41..35e35d307193 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -33,7 +33,7 @@ export default async function render_route(request, route) { ); } - let { status = 200, body = '', headers = {} } = response; + let { status = 200, body = {}, headers = {} } = response; headers = lowercase_keys(headers); const type = headers['content-type']; diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js index bdf5cf7ab34b..6a1d9c42ad0e 100644 --- a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js @@ -9,6 +9,7 @@ export default function (test) { test.only('200 status on empty endpoint', null, async ({ fetch }) => { const res = await fetch('/endpoint-output/empty'); assert.equal(res.status, 200); + assert.equal(await res.json(), {}); }); test.only('set-cookie without body', null, async ({ fetch }) => { From 02fdeec3dd44de33dfc63f33b5cc4c7c3a77553e Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 2 Jul 2021 22:47:33 +0700 Subject: [PATCH 05/11] test: success status when body is defined --- .../test/apps/basics/src/routes/endpoint-output/_tests.js | 6 ++++++ .../kit/test/apps/basics/src/routes/endpoint-output/body.js | 4 ++++ 2 files changed, 10 insertions(+) create mode 100644 packages/kit/test/apps/basics/src/routes/endpoint-output/body.js diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js index 6a1d9c42ad0e..3a8984606c9d 100644 --- a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js @@ -17,4 +17,10 @@ export default function (test) { assert.equal(res.status, 200); assert.equal(res.headers.has('set-cookie'), true); }); + + test.only('200 status by default', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/body'); + assert.equal(res.status, 200); + assert.equal(await res.text(), '{}'); + }); } diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/body.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/body.js new file mode 100644 index 000000000000..6cafc71b6f6f --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/body.js @@ -0,0 +1,4 @@ +/** @type {import('@sveltejs/kit').RequestHandler} */ +export function get() { + return { body: {} }; +} From aeec616130a324bc872d7569106e92354273438e Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 2 Jul 2021 22:49:32 +0700 Subject: [PATCH 06/11] remove only flags --- .../test/apps/basics/src/routes/endpoint-output/_tests.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js index 3a8984606c9d..32513a3a3fcf 100644 --- a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js @@ -2,23 +2,23 @@ import * as assert from 'uvu/assert'; /** @type {import('test').TestMaker} */ export default function (test) { - test.only('not ok on void endpoint', null, async ({ fetch }) => { + test('not ok on void endpoint', null, async ({ fetch }) => { const res = await fetch('/endpoint-output/empty', { method: 'DELETE' }); assert.equal(res.ok, false); }); - test.only('200 status on empty endpoint', null, async ({ fetch }) => { + test('200 status on empty endpoint', null, async ({ fetch }) => { const res = await fetch('/endpoint-output/empty'); assert.equal(res.status, 200); assert.equal(await res.json(), {}); }); - test.only('set-cookie without body', null, async ({ fetch }) => { + test('set-cookie without body', null, async ({ fetch }) => { const res = await fetch('/endpoint-output/headers'); assert.equal(res.status, 200); assert.equal(res.headers.has('set-cookie'), true); }); - test.only('200 status by default', null, async ({ fetch }) => { + test('200 status by default', null, async ({ fetch }) => { const res = await fetch('/endpoint-output/body'); assert.equal(res.status, 200); assert.equal(await res.text(), '{}'); From b5e04ca51c31307d9633a22674f0ef8f6ad54326 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Sat, 3 Jul 2021 00:06:58 +0700 Subject: [PATCH 07/11] add changeset --- .changeset/fluffy-cheetahs-remain.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fluffy-cheetahs-remain.md diff --git a/.changeset/fluffy-cheetahs-remain.md b/.changeset/fluffy-cheetahs-remain.md new file mode 100644 index 000000000000..bf4424eb6222 --- /dev/null +++ b/.changeset/fluffy-cheetahs-remain.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +handle undefined body on endpoint output From 4e3284e4c51a0f62dc46688a1a72a49af1c94709 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Wed, 7 Jul 2021 08:14:46 +0700 Subject: [PATCH 08/11] test other method does not throw as well --- .../basics/src/routes/endpoint-output/_tests.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js index 32513a3a3fcf..a65e96fe43b8 100644 --- a/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js +++ b/packages/kit/test/apps/basics/src/routes/endpoint-output/_tests.js @@ -23,4 +23,17 @@ export default function (test) { assert.equal(res.status, 200); assert.equal(await res.text(), '{}'); }); + + test('does not throw on blob method', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/empty'); + assert.type(await res.blob(), 'object'); + }); + test('does not throw on arrayBuffer method', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/empty'); + assert.type(await res.arrayBuffer(), 'object'); + }); + test('does not throw on buffer method', null, async ({ fetch }) => { + const res = await fetch('/endpoint-output/empty'); + assert.type(await res.buffer(), 'object'); + }); } From e04b6ab98292340c61bba179824c5b966b0b9f6b Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Sat, 10 Jul 2021 07:03:09 +0700 Subject: [PATCH 09/11] move body substitution only for JSON type --- packages/kit/src/runtime/server/endpoint.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 35e35d307193..f10f74ef02cb 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -33,7 +33,7 @@ export default async function render_route(request, route) { ); } - let { status = 200, body = {}, headers = {} } = response; + let { status = 200, body, headers = {} } = response; headers = lowercase_keys(headers); const type = headers['content-type']; @@ -54,9 +54,12 @@ export default async function render_route(request, route) { /** @type {import('types/hooks').StrictBody} */ let normalized_body; - if (typeof body === 'object' && (!type || type === 'application/json')) { + if ( + (typeof body === 'object' || typeof body === 'undefined') && + (!type || type === 'application/json') + ) { headers = { ...headers, 'content-type': 'application/json' }; - normalized_body = JSON.stringify(body); + normalized_body = JSON.stringify(body || {}); } else { normalized_body = /** @type {import('types/hooks').StrictBody} */ (body); } From b3e860fe6c62461eae0f13f906e6bd8a9e25c6e3 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Sat, 10 Jul 2021 07:06:47 +0700 Subject: [PATCH 10/11] refactor error messages --- packages/kit/src/runtime/server/endpoint.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index f10f74ef02cb..7c810f9de05c 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -25,12 +25,11 @@ export default async function render_route(request, route) { const params = route.params(match); const response = await handler({ ...request, params }); + const preface = `Invalid response from route ${request.path}`; if (response) { if (typeof response !== 'object') { - return error( - `Invalid response from route ${request.path}: expected an object, got ${typeof response}` - ); + return error(`${preface}: expected an object, got ${typeof response}`); } let { status = 200, body, headers = {} } = response; @@ -41,13 +40,13 @@ export default async function render_route(request, route) { // validation if (type === 'application/octet-stream' && !(body instanceof Uint8Array)) { return error( - `Invalid response from route ${request.path}: body must be an instance of Uint8Array if content type is application/octet-stream` + `${preface}: body must be an instance of Uint8Array if content type is application/octet-stream` ); } if (body instanceof Uint8Array && type !== 'application/octet-stream') { return error( - `Invalid response from route ${request.path}: Uint8Array body must be accompanied by content-type: application/octet-stream header` + `${preface}: Uint8Array body must be accompanied by content-type: application/octet-stream header` ); } From 3db0b35c59061ac48f7d685307851b92c22677f0 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Sat, 10 Jul 2021 07:35:55 +0700 Subject: [PATCH 11/11] oops --- packages/kit/src/runtime/server/endpoint.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/server/endpoint.js b/packages/kit/src/runtime/server/endpoint.js index 789f97590235..c3349929da08 100644 --- a/packages/kit/src/runtime/server/endpoint.js +++ b/packages/kit/src/runtime/server/endpoint.js @@ -57,7 +57,7 @@ export default async function render_route(request, route) { (typeof body === 'object' || typeof body === 'undefined') && (!type || type.startsWith('application/json')) ) { - headers = { ...headers, 'content-type': 'application/json' }; + headers = { ...headers, 'content-type': 'application/json; charset=utf-8' }; normalized_body = JSON.stringify(body || {}); } else { normalized_body = /** @type {import('types/hooks').StrictBody} */ (body);