Validate handle response

[PH4NTOMiki]

Describe the bug

I upgraded the adapter-node and kit, and changed the hooks to reflect new syntax from #3384 and now the server crashes when started with the exit code in log

Reproduction

https://github.com/PH4NTOMiki/sveltekit-book

Logs

file:///home/mutic/dev/sveltekit-book/node_modules/@sveltejs/kit/dist/node.js:64
        res.writeHead(response.status, Object.fromEntries(response.headers));
                                              ^

TypeError: object is not iterable (cannot read property Symbol(Symbol.iterator))
    at Function.fromEntries (<anonymous>)
    at setResponse (file:///home/mutic/dev/sveltekit-book/node_modules/@sveltejs/kit/dist/node.js:64:40)
    at file:///home/mutic/dev/sveltekit-book/node_modules/@sveltejs/kit/dist/chunks/index.js:2083:8
    at processTicksAndRejections (node:internal/process/task_queues:96:5)

Node.js v17.2.0

System Info

System:
    OS: Linux 5.13 Manjaro Linux
    CPU: (8) x64 Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz
    Memory: 1.28 GB / 7.65 GB
    Container: Yes
    Shell: 5.1.12 - /bin/bash
  Binaries:
    Node: 17.2.0 - /usr/local/bin/node
    npm: 8.1.4 - /usr/bin/npm
  Browsers:
    Firefox: 95.0.1
  npmPackages:
    @sveltejs/adapter-node: ^1.0.0-next.66 => 1.0.0-next.66 
    @sveltejs/kit: ^1.0.0-next.236 => 1.0.0-next.236 
    svelte: ^3.44.3 => 3.44.3

Severity

blocking an upgrade

Additional Information

Everything was working without any issue before update.

[benmccann]

I'm guessing it's this line:

kit/packages/kit/src/node.js

Line 64 in 1bce8e9

res.writeHead(response.status, Object.fromEntries(response.headers));

[Rich-Harris]

Can you provide more details about how to reproduce this? Your repro is working fine for me

[PH4NTOMiki]

Actually for me it works sometimes, and sometimes not, (with and without the entries() fix).
In some starts the server works, and in some it just doesn't, I don't understand what's going on

[PH4NTOMiki]

It's like 10:4 ratio

[PH4NTOMiki]

I added a console log to my dist/node.js file, and I'll let you know if i get something from it.

[PH4NTOMiki]

@Rich-Harris Actually I found out what is the error, and event the log shows it, sometimes the response.headers is an object(plain object, not Headers instance), and that's why when the Object.fromEntries is called with object, the error is thrown, the fix should be like this:

res.writeHead(response.status, response.headers.constructor.toString().indexOf('Headers') > -1 ? Object.fromEntries(response.headers)) : response.headers;

[PH4NTOMiki]

@Rich-Harris Actually I found out what is the error, and event the log shows it, sometimes the response.headers is an object(plain object, not Headers instance), and that's why when the Object.fromEntries is called with object, the error is thrown, the fix should be like this:

res.writeHead(response.status, response.headers.constructor.toString().indexOf('Headers') > -1 ? Object.fromEntries(response.headers)) : response.headers;

Something like that, I used constructor to check if it's a Headers but could be(probably there is) something better

[Conduitry]

A better fix would be to figure out why it's sometimes getting passed an object. Can you use console.trace to get a stacktrace and post that for one of the times that it's a plain object instead of a Headers instance?

[PH4NTOMiki]

A better fix would be to figure out why it's sometimes getting passed an object. Can you use console.trace to get a stacktrace and post that for one of the times that it's a plain object instead of a Headers instance?

Yeah, I understand and agree with you, but it doesn't happen frequently now for me, if it happens I'll write here, but for me that's seems like a quick and easy fix for the issue, that occurs rarely.

[Rich-Harris]

Are you returning a headers POJO from a custom handle hook? That could explain it (and we should catch any non-Response return values from handle and throw an error)

[PH4NTOMiki]

Yeah, you are right, actually I figured out that myself a few minutes ago and fixed it with a commit https://github.com/PH4NTOMiki/sveltekit-book/commit/e673854ab7f550e7bf3ff7ab87481e9f7b2a7259 to my repo, it was leftover from the previous version, and it was in a conditional that rarely comes true(it was only when the user is logged out and tried to access the API route) and when I figured out that it errors only when I'm logged-out and I knew where to look

[gterras]

I met the same error, for those searching for a fix I had to change :

export const handle = async ({ event, resolve }) => {
	if (condition) 
		return LoginPage
}

const LoginPage = {
	status: 200,
	headers: {
		'content-type': 'text/html',
		'cache-Control': 'private, max-age=0, no-cache',
	},
	body: '<html>...</html>',
}

to :

const LoginPage = () => new Response('<html>...</html>', {
	status: 200,
	headers: {
		'content-type': 'text/html',
		'cache-Control': 'private, max-age=0, no-cache',
	}
})