fix: 9475 adapter node log request error

[dreitzner]

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Fixes #9475
This change will additionally log an error if the getRequest function in the SSR handler fails.
I don't think we should test for an error log 😉
Will log an error similar to:

[changeset-bot]

🦋 Changeset detected

Latest commit: dab875d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/adapter-node	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[tcc-sejohnson]

I haven't had time to dig into this, just wanted to point out that you should add a test to assert that the warning is being logged. Here's how:

kit/packages/kit/test/apps/basics/test/test.js

Lines 830 to 844 in b48d58e

	test.describe('Actions', () => {
	test('Submitting a form with a file input but no enctype="multipart/form-data" logs a warning', async ({
	page,
	javaScriptEnabled
	}) => {
	test.skip(!javaScriptEnabled, 'Skip when JavaScript is disabled');
	test.skip(!process.env.DEV, 'Skip when not in dev mode');
	await page.goto('/actions/file-without-enctype');
	const log_promise = page.waitForEvent('console');
	await page.click('button');
	const log = await log_promise;
	expect(log.text()).toBe(
	'Your form contains <input type="file"> fields, but is missing the `enctype="multipart/form-data"` attribute. This will lead to inconsistent behavior between enhanced and native forms. For more details, see https://github.com/sveltejs/kit/issues/9819. This will be upgraded to an error in v2.0.'
	);
	});

[benmccann]

this change could be considered a feature or a fix. I'm not sure it's big enough for a new minor release, so a patch release is probably more appropriate

Suggested change

	'@sveltejs/adapter-node': minor
	'@sveltejs/adapter-node': patch

[benmccann]

we prefix messages with fix: or feat:. the whole changelog is for adapter-node, so noting that again wouldn't add anything

Suggested change

	Adapter Node: log getRequest error
	fix: log error details when invalid request body is encountered

[benmccann]

My question with this is whether it would get annoying and fill the logs. I'm not that familiar with how logging is configured in JavaScript. This category of issue doesn't indicate any issue with the application and so I'm not sure that error is the appropriate console level to use here.

[symbolicsorcerer]

I've had a lot of trouble debugging an app running with the node adapter. There's literally nothing logged aside from the initial "listening" message. I think we need to go even further than this PR, but this is at least a good start to handle one kind of error condition. I've been googling for quite a while to better understand how to troubleshoot adapter-node apps and have come up empty-handed, and also tried asking on the Discord.

When you have a production application deployed with adapter-node, at some point you're going to want some logging for later troubleshooting. If you want to keep the noise down, we could use a log level or env var. I'm not quite sure what the typical JS idioms are here either, but I don't think complete silence is reasonable.

[Rich-Harris]

This (and #9475) can now be closed — since #11289 (i.e. SvelteKit 2), creating the request will always succeed, and if there's an invalid body we'll find out when we try and read it:

export async function POST({ request }) {
  // if the request body is invalid, this will fail, and `handleError` will be invoked
  const body = await request.json();

  // ...
}

There is a small issue in that handleError is called with a status and message of 500/Internal Error rather than 413/Payload Too Large — this is because of a failed instanceof check resulting from some duplicated code — but the error itself contains the relevant details. Will open a separate issue for that.