feat: event.isSubRequest

[Conduitry]

Closes #10163. No test yet because I want to first get some feedback on whether this seems like the correct approach. I think state.depth > 0 will tell us what we want. Also, if anyone has a better idea for a name than event.isSyntheticRequest, I would love to hear it.

Edit: I confirmed with Rich that this seemed like a sensible approach. I added tests for this, which I had to scale back a little in scope, because prerendering was making it hard to have another way of knowing whether this is a synthetic request.

Finally, I'm starting to really dislike the name 'synthetic request', because there are other types of requests (during prerendering) that are for pages, and thus aren't the type of request that this flag is for. I only want to be identifying requests that are sub-requests from fetches during SSR. Heck if I have a good name for that though.

Edit again: I think we've settled on isSubRequest, which isn't awful, at least.

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:.

[changeset-bot]

🦋 Changeset detected

Latest commit: 9ec2a33

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

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	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

[Conduitry]

I've pushed a test similar to the one for isDataRequest. It adds a handle middleware to the basic tests app that makes sure that the user-agent header is set, or isSyntheticRequest is true, but not both. The absence of the user-agent header seemed like a reasonable way to approximate the situations in which we're receiving a synthetic request.

[Conduitry]

I don't know what's happening with the tests here.

[Conduitry]

I think the latest round of failures may indicate a bug in my implementation here. In the test that navigates to /errors/error-in-layout, the layout is making a load fetch to /errors/error-in-layout/non-existent, and apparently that doesn't have the isSyntheticRequest set.

[Conduitry]

Ah. Nope. It's a bad test. In that particular case, since the (synthetic) call to respond() can't find the route in the manifest, so it makes a real network call. However, that real network call still has the x-this-is-a-synthetic-request header in it that I previously injected into the (synthetic) network call earlier in handleFetch.

I'm going to have to find a different way to test this. I had initially tried looking for user-agent in the requests, and if that was missing, assuming it was a synthetic request, and that I should require that isSyntheticRequest be set. However, this ran into issues with prerendering, where that header is also missing even for the initial request coming in. I'll have to find some other mechanism.

(This does also make me wonder whether 'synthetic' is the right word. Arguably all of the requests made during prerendering are synthetic, but that's not what I'm trying to capture here. I want to know whether this is an internal fetch during SSR. Naming things is hard.)

[Conduitry]

Okay, the tests should be good to go now. I gave up on trying to detect whether a prerendering request has isSyntheticRequest set correctly, because that seemed to be too complicated to check for. So, if the Host header is present, then I'm making sure the User-Agent is set if and only if isSyntheticRequest is not set.

I do still think the isSyntheticRequest name isn't a good name, though, because there are other types of synthetic requests (e.g., during prerendering) that we're not talking about here. I don't know what to call the type of request we're indicating with this flag.

[Conduitry]

Whee tests are failing. It looks like it's the VERSION thing you mentioned in Discord?

[dummdidumm]

If you merge master into this and increase the bug version by one then it should go away 🙏

[Conduitry]

I've merged master in, but I don't know what "increase the bug version by one" means.

[dummdidumm]

The patch version, sry. 10.20.4->10.20.5 in version.js or whatever the current version is . I can do it in a few minutes when I'm back at my laptop

[dummdidumm]

Oh it seems like Ben fixed it on master already