hooks.js example causes a build error #763

websocket98765 · 2021-03-30T01:11:55Z

Describe the bug

When the following hooks.js file exists, npm run build throws an error. (However npm run dev works without error and the site runs perfectly.)

To Reproduce

Add src/hooks.js

export async function handle(request, render) {
  const response = await render(request);

  return {
    ...response,
    headers: {
      ...response.headers,
      'x-custom-header': 'potato'
    }
  };
}

Run npm run build.

The command will throw this error:

✓ 38 modules transformed.
.svelte/output/server/app.js      193.07kb
.svelte/output/server/style.css   7.76kb

Run npm start to try your app locally.

> Using @sveltejs/adapter-node
TypeError: Cannot read property 'headers' of undefined
    at Object.handle (file:///Users/me/sveltekit-demo/.svelte/output/server/app.js:1890:19)
    at async ssr (file:///Users/me/sveltekit-demo/.svelte/output/server/app.js:1677:12)
    at async visit (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:531:20)
    at async prerender (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:682:5)
    at async Builder.prerender (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:743:4)
    at async adapt (/Users/me/sveltekit-demo/node_modules/@sveltejs/adapter-node/index.js:26:4)
    at async adapt (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:765:2)
    at async file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/cli.js:598:5
> 500 /about
Error: 500 /about
    at error (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:523:11)
    at visit (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:574:5)
    at async prerender (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:682:5)
    at async Builder.prerender (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:743:4)
    at async adapt (/Users/me/sveltekit-demo/node_modules/@sveltejs/adapter-node/index.js:26:4)
    at async adapt (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:765:2)
    at async file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/cli.js:598:5
npm ERR! code 1
npm ERR! path /Users/me/sveltekit-demo
npm ERR! command failed
npm ERR! command sh -c svelte-kit build

about.svelte is nothing special:

<h1>About this site</h1>
<p>TODO...</p>

svelte.config.cjs contains adapter as: adapter: node(),.

Information about your SvelteKit Installation:

  System:
    OS: macOS 11.2.2
    CPU: (8) arm64 Apple M1
    Shell: 5.8 - /bin/zsh
 npmPackages:
    @sveltejs/kit: next => 1.0.0-next.64 
    svelte: ^3.35.0 => 3.35.0

Severity

Minor

The text was updated successfully, but these errors were encountered:

Rich-Harris · 2021-03-30T01:36:41Z

Ah, yep, this makes sense. In the prerender step, nothing gets returned from render for non-prerenderable pages. Not 100% sure what the right move is here — it's easy enough to just add an if (response) block but it feels a bit leaky since something is always returned normally. It could return a 404 response but that doesn't feel totally correct either. Hmm...

tbremer · 2021-04-02T04:31:03Z

Ran across an issue with the prerender step because I have a handle function that is doing some basic checks to ensure only authorized users can access a page.

hooks.ts:

import { ensureAdmin } from '$lib/ensureAdmin.ts';

export async function handle(request, render) {
	const { path, headers } = request;

	if (/\/admin/.test(path) && !ensureAdmin(request)) {
		return { status: 403 };
	}

	return render(request);
}

I tried looking at the different environments from $app/env but it did not seem appropriate to key off of any of the current ones.

Maybe if there was an env for prerendering / adapting?

Rich-Harris · 2021-04-02T14:02:00Z

Yeah, I think import { prerender } from '$app/env' is a great idea

tbremer · 2021-04-02T17:03:28Z

I was poking around in the code adjacent to the import.meta.env and it seems like most of that is hoisted from the Vite process. Vite exposes additionally MODE flag which is production for the build (https://github.com/vitejs/vite/blob/77971eda77db3cdaf07880bf19846b184de62466/docs/guide/env-and-mode.md), but that is not entirely helpful for this issue.

If I am grokking the codebase correctly it seems like this would be the first env flag that would be unique to SvelteKit runtime.

It looks like the adapt function could be a good place to update that env flag.

kit/packages/kit/src/core/adapt/index.js

Lines 9 to 19 in 8805c6d

    
           export async function adapt(config, { cwd = process.cwd(), verbose }) { 
        
           	const { name, adapt } = config.kit.adapter; 
        
           	console.log(colors.bold().cyan(`\n> Using ${name}`)); 
        
           	const log = logger({ verbose }); 
        
           	const builder = new Builder({ cwd, config, log }); 
        
           	await adapt(builder); 
        
           	log.success('done'); 
        
           }

Though, it does not look like the $app/env exports are reactive, so it might be better placed in the CLI build function.

kit/packages/kit/src/cli.js

Lines 100 to 104 in 24fab19

    
           prog 
        
           	.command('build') 
        
           	.describe('Create a production build of your app') 
        
           	.option('--verbose', 'Log more stuff', false) 
        
           	.action(async ({ verbose }) => {

Edit: Sorry for blasting this issue, but I did a little more digging and found that Vite does some pretty heavy handed work for ensuring the import.meta.env is defined within their code.

https://github.com/vitejs/vite/blob/fa8574921195dd03b539c150a2ae5f97121a0aea/packages/vite/src/node/plugins/define.ts#L16-L31

Any thoughts? I'm happy to make a pull request.

Rich-Harris · 2021-04-02T19:06:27Z

In the process of trying to figure out how we'd tackle this, I ended up writing the code, so I took the liberty of opening a PR: #833

tbremer · 2021-04-02T19:55:35Z

That's awesome, thanks for taking the time to move this along!

* add prerendering to $app/env (#763) * add docs

benmccann · 2021-04-02T23:36:50Z

I'm assuming this was fixed since #833 was merged, but let me know if that's incorrect. I was having a bit of a hard time following whether there were two things being discussed here or just one

acoyfellow · 2021-04-03T00:19:59Z

I have to investigate if my problem is actually this... but, i updated to ^1.0.0-next.69 and this error persists for me (TypeError: Cannot read property 'headers' of undefined). My hooks file i'm using is this: https://gist.github.com/acoyfellow/d8e86979c66ebea25e1643594e38be73

edit: I can still reproduce with this simple hooks/index.js file:

export async function handle(request, render) {
  const response = await render(request);
  console.log('response', response);
  return {
    ...response,
    headers: {
      ...response.headers
    }
  };
}

error:

response undefined
TypeError: Cannot read property 'headers' of undefined

Rich-Harris · 2021-04-03T00:44:44Z

yeah, the issue isn't fixed, #833 just enables a workaround. There's a couple of solutions I can think of:

Document that render may not return a response if we're prerendering
Return a simple response with a 204 code for 'this page was not prerenderable', and attach a header that the prerenderer understands so that it knows a) not to write anything to disk and b) not to fail the build

I think I prefer 2:

if (!page_config.prerender) {
  return {
    status: 204,
    headers: {
      'x-svelte-kit-prerendered': 'false'
    }
  };
}

Now that I think about it, using a header like this would be a neat way to avoid the undocumented dependencies array that accompanies some page responses (indicating which resources were fetched and need to be emitted as static assets). That would prevent the breakage I foresee from people doing this:

export async function handle(request, render) {
  const response = await render(request);
  return {
    status: response.status,
    headers: response.headers,
    body: transform(response.body)
    // oops, omitted the 'dependencies' array, you just broke prerendering
  };
}

Is x- an appropriate prefix to use for these sorts of headers?

Conduitry · 2021-04-03T00:50:23Z

Yeah, having an undocumented top-level key in the response object that people need to carry along without knowing about seems risky.

Sticking that somewhere in headers seems safer, as people are more likely to use a rest/spread there to just carry along the headers they don't care about.

For these secret API things, what about using a key name that's not a valid HTTP header name, just to make it clear that something special is happening here, and to avoid any sort of conflict?

Rich-Harris · 2021-04-03T00:58:12Z

According to https://stackoverflow.com/a/3569667, which is a summary of the relevant IETF RFC, the following characters are invalid HTTP header names:

( ) < > @ , ; : \ " / [ ] ? = { }

So we could do something like @sveltekit-prerender: false and @sveltekit-dependencies: {...}

Conduitry · 2021-04-03T01:02:12Z

That sounds reasonable to me. 👍

acoyfellow · 2021-04-03T11:27:22Z

Sorry to be annoying, but I'm still getting the same error with @sveltejs/kit@^1.0.0-next.70 + @sveltejs/adapter-node@^1.0.0-next.12

exmple hooks/index.js file:

export async function handle(request, render) {
  const response = await render(request);
  console.log('response', response);
  return {
    ...response,
    headers: {
      ...response.headers
    }
  };
}

shows:

Run npm start to try your app locally.

> Using @sveltejs/adapter-node
response undefined
TypeError: Cannot read property 'headers' of undefined

websocket98765 · 2021-04-03T13:47:21Z

Similar to the above comment, I get an error about headers now when running npm run build:

> Using @sveltejs/adapter-node
TypeError: Cannot read property 'headers' of undefined
    at Object.handle (file:///Users/me/sveltekit-demo/.svelte/output/server/app.js:1924:19)
    at async ssr (file:///Users/me/sveltekit-demo/.svelte/output/server/app.js:1711:12)
    at async visit (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:531:20)
    at async prerender (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:682:5)
    at async Builder.prerender (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:743:4)
    at async adapt (/Users/me/sveltekit-demo/node_modules/@sveltejs/adapter-node/index.js:26:4)
    at async adapt (file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/chunks/index5.js:765:2)
    at async file:///Users/me/sveltekit-demo/node_modules/@sveltejs/kit/dist/cli.js:604:5
> 500 /about
Error: 500 /about

Using:

SvelteKit: 1.0.0-next.70
adapter-node: 1.0.0-next.12

acoyfellow · 2021-04-04T15:03:19Z

Can confirm that this is still happening on 1.0.0-next.71.

This issue should unlock the first steps to the CSP issue I think. I haven't tackled the nonce functionality, but we'd be able to set other headers that are important for security

DhyeyMoliya · 2021-04-05T12:32:39Z

I am also encountering this issue. It happens at build time. The functions getContext, getSession and handle are being called after build. And I don't have export const prerender = true; in any routes. Don't know why it should run these functions if there is no pre-rendering specified in any route.

One more side-effect of this issue. I am initializing my DB Connection at the top of the hooks.ts file. So, after svelte-kit build, my build process does not quit/exit, because the db connection stays open and prevents the process from exiting (maybe). I am also trying to execute some post-build scripts. But since the process is not exiting, my scripts are not starting 😄.

acoyfellow · 2021-04-06T23:42:56Z

I'm happy to report that i'm now able to build.

Same version.. So something I was doing was triggering this.

I don't know enough about the internals of SK, but I have a hunch it has to do with this bug - because once I cleaned up all the instances of this, I tried the hooks handle() again, and it built.

websocket98765 · 2021-04-07T15:21:58Z

Same. Seems to be fixed in "@sveltejs/kit": "^1.0.0-next.71",.
(Builds successfully and site works with custom headers in both dev and production.)

vaib1343 · 2022-01-29T19:17:25Z

config.kit.ssr has been removed — use the handle hook instead: https://kit.svelte.dev/docs#hooks-handle.
how can i resolve this issue

berga · 2022-02-07T09:51:10Z

config.kit.ssr has been removed — use the handle hook instead: https://kit.svelte.dev/docs#hooks-handle. how can i resolve this issue

hi, I've solved restarting from the svelte@next boilerplate:
hooks.ts

// import cookie from 'cookie';
// import { v4 as uuid } from '@lukeed/uuid';
import type { Handle } from '@sveltejs/kit';

export const handle: Handle = async ({ event, resolve }) => {
	// const cookies = cookie.parse(event.request.headers.get('cookie') || '');
	// event.locals.userid = cookies.userid || uuid();

	const response = await resolve(event, {ssr: false});

	// if (!cookies.userid) {
	// 	// if this is the first time the user has visited this app,
	// 	// set a cookie so that we recognise them when they return
	// 	response.headers.set(
	// 		'set-cookie',
	// 		cookie.serialize('userid', event.locals.userid, {
	// 			path: '/',
	// 			httpOnly: true
	// 		})
	// 	);
	// }

	return response;
};

Rich-Harris added the bug Something isn't working label Mar 30, 2021

acoyfellow mentioned this issue Mar 31, 2021

Content Security Policy support #93

Closed

Rich-Harris added a commit that referenced this issue Apr 2, 2021

add prerendering to $app/env (#763)

48656ed

Rich-Harris mentioned this issue Apr 2, 2021

Add prerendering to $app/env #833

Merged

5 tasks

Rich-Harris pushed a commit that referenced this issue Apr 2, 2021

Add prerendering to $app/env (#833)

4d2cd62

* add prerendering to $app/env (#763) * add docs

benmccann closed this as completed Apr 2, 2021

Rich-Harris reopened this Apr 3, 2021

Rich-Harris mentioned this issue Apr 3, 2021

Always return something from render #847

Merged

5 tasks

Rich-Harris closed this as completed in #847 Apr 3, 2021

benmccann reopened this Apr 4, 2021

websocket98765 closed this as completed Apr 7, 2021

benmccann mentioned this issue Apr 12, 2021

Stop / shutdown hook to close database connections, etc. #978

Closed

stephane-vanraes mentioned this issue Apr 23, 2021

Pages are rendered during build despite not setting prerender = true #1188

Closed

ciearius mentioned this issue Jul 6, 2021

Deploy new approach GeneralMine/S2QUAT#36

Closed

maechel mentioned this issue Mar 14, 2022

config.kit.ssr has been removed stevekinney/cypress-fem#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

hooks.js example causes a build error #763

hooks.js example causes a build error #763

websocket98765 commented Mar 30, 2021

Rich-Harris commented Mar 30, 2021

tbremer commented Apr 2, 2021

Rich-Harris commented Apr 2, 2021

tbremer commented Apr 2, 2021 •

edited

Loading

Rich-Harris commented Apr 2, 2021

tbremer commented Apr 2, 2021

benmccann commented Apr 2, 2021

acoyfellow commented Apr 3, 2021 •

edited

Loading

Rich-Harris commented Apr 3, 2021

Conduitry commented Apr 3, 2021

Rich-Harris commented Apr 3, 2021

Conduitry commented Apr 3, 2021

acoyfellow commented Apr 3, 2021 •

edited

Loading

websocket98765 commented Apr 3, 2021

acoyfellow commented Apr 4, 2021 •

edited

Loading

DhyeyMoliya commented Apr 5, 2021 •

edited

Loading

acoyfellow commented Apr 6, 2021

websocket98765 commented Apr 7, 2021 •

edited

Loading

vaib1343 commented Jan 29, 2022

berga commented Feb 7, 2022

hooks.js example causes a build error #763

hooks.js example causes a build error #763

Comments

websocket98765 commented Mar 30, 2021

Rich-Harris commented Mar 30, 2021

tbremer commented Apr 2, 2021

Rich-Harris commented Apr 2, 2021

tbremer commented Apr 2, 2021 • edited Loading

Rich-Harris commented Apr 2, 2021

tbremer commented Apr 2, 2021

benmccann commented Apr 2, 2021

acoyfellow commented Apr 3, 2021 • edited Loading

Rich-Harris commented Apr 3, 2021

Conduitry commented Apr 3, 2021

Rich-Harris commented Apr 3, 2021

Conduitry commented Apr 3, 2021

acoyfellow commented Apr 3, 2021 • edited Loading

websocket98765 commented Apr 3, 2021

acoyfellow commented Apr 4, 2021 • edited Loading

DhyeyMoliya commented Apr 5, 2021 • edited Loading

acoyfellow commented Apr 6, 2021

websocket98765 commented Apr 7, 2021 • edited Loading

vaib1343 commented Jan 29, 2022

berga commented Feb 7, 2022

tbremer commented Apr 2, 2021 •

edited

Loading

acoyfellow commented Apr 3, 2021 •

edited

Loading

acoyfellow commented Apr 3, 2021 •

edited

Loading

acoyfellow commented Apr 4, 2021 •

edited

Loading

DhyeyMoliya commented Apr 5, 2021 •

edited

Loading

websocket98765 commented Apr 7, 2021 •

edited

Loading