Post requests to page endpoints seem to call the get handler as well: it's either a bug or undocumented

[cdcarson]

Describe the bug

When you have a page endpoint with both a get method and a post method, a POST request will result in the get method being called after the post method, with the response bodies from both methods being merged.

This may be the intended (though confusing and potentially costly) behavior. If so, it isn't documented, as far as I can tell. I'm looking here:

These functions can, like get, return a body that will be passed to the page as props. Whereas 4xx/5xx responses from get will result in an error page rendering, similar responses to non-GET requests do not, allowing you to do things like render form validation errors[...]

There's nothing I see about get being called post, or the order in which things are merged (although this seems ok, with the result of the post not being entirely overwritten.)

It's unclear whether this merging occurs with the response headers as well. (I tried but failed to figure this out in my repo below.)

Ideally, for me, this wouldn't happen at all. Yeah, maybe a few lines of code can be saved by the inheritance, but it's at the cost of

• clarity and
• making repetitive api/database calls (when I update a row in my db it returns the row -- why do I need to fetch it again in get? At that point a lot of the benefit of page endpoints goes away.)

If it is the intended behavior, then it should be documented so people can work around it, probably by avoiding page endpoints altogether.

But page endpoint methods are nice, and I'd like to keep using them, if possible. The happy paradigm should be that each operates independently, being responsible for returning the full set of route component properties.

Reproduction

A minimal reproduction of the behavior is here:
https://github.com/cdcarson/sk-issue-get-handler-called-after-post-handler

git clone https://github.com/cdcarson/sk-issue-get-handler-called-after-post-handler.git
cd sk-issue-get-handler-called-after-post-handler.git
npm i
npm run dev -- --open

Open the site, submit the form on the home page, and look at what the server logs in the console. You will see...

  SvelteKit v1.0.0-next.344

  local:   http://localhost:3000
  network: not exposed

  Use --host to expose server to other devices on this network


--- Start of request --
In get handler. The request method is: GET
In time consuming getKittenName api call.
--- End of request --

--- Start of request --
In post handler. The request method is: POST
In time consuming updateKittenName api call.
In get handler. The request method is: POST
In time consuming getKittenName api call.
--- End of request --

For completeness it also happens in prod (I'm using the node adapter)...

npm run build
npm run preview

Logs

No response

System Info

System:
    OS: macOS 12.4
    CPU: (8) arm64 Apple M1
    Memory: 75.08 MB / 8.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.13.2 - ~/.nvm/versions/node/v16.13.2/bin/node
    npm: 8.1.2 - ~/.nvm/versions/node/v16.13.2/bin/npm
  Browsers:
    Chrome: 101.0.4951.64
    Safari: 15.5
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.48 
    @sveltejs/adapter-node: ^1.0.0-next.76 => 1.0.0-next.76 
    @sveltejs/kit: next => 1.0.0-next.344 
    svelte: ^3.44.0 => 3.48.0

Severity

serious, but I can work around it

Additional Information

No response

[Conduitry]

This is intended, as it's (generally) the more expected behavior. I believe there has been at least one issue opened previously about this, but I couldn't immediately find it. If you want your GET page endpoint to (effectively) be a no-op, you can have it bail if event.request.method !== 'GET'. This could all probably be documented better.

[elliott-with-the-longest-name-on-github]

Thank you for the detailed repro!

One thing I can say for sure is that this is intended behavior. See the issue that spawned page endpoints (called "shadow endpoints" during development): #3532 - Rich lays out this behavior in the second(?) section.

Keep in mind, this is all done in one roundtrip to the server -- you're not making a networked POST call, receiving the result, and then making a networked GET call, with the bodies being merged.

@Conduitry's workaround absolutely works, but from a semantic perspective, if you want a POST endpoint separate from the GET one, I would just use a dedicated endpoint (eg. /foo.svelte, /foo.json.js).

Absolutely agreed that this could be documented better.

[cdcarson]

Aah. Ok. Thanks. One question -- what's the proper way to bail? With nothing or with {status: 200}?

[babichjacob]

I'm looking here:

By, the way, right under that paragraph is this code snippet:

<!-- src/routes/items.svelte -->
<script>
  // The page always has access to props from `get`...  
  export let items;
  // ...plus props from `post` when the page is rendered
  // in response to a POST request, for example after
  // submitting the form below
  export let errors;
</script>

[cdcarson]

Ok, thanks @Conduitry @tcc-sejohnson and @babichjacob (special props for pointing out that I never ever read anybody else's code comments, sorry about that. 👍 )

To expand on what @Conduitry said about bailing...

If you want your GET page endpoint to (effectively) be a no-op, you can have it bail if event.request.method !== 'GET'.

...this is how to bail...

export const get: RequestHandler = async (event) => {
  if (event.request.method !== 'GET') {
    return {};
  }
  console.log('In get handler. The request method is:', event.request.method);
  const kittenName = await getKittenName();
  return {
    status: 200,
    body: {
      kittenName,
      testIfPropOverriddenInGet: 'set in get method',
      method: event.request.method
    }

  }
}

This keeps the response just as it was returned by the post handler.

I'm fine with closing this issue, but I'll leave it up to you all. Thanks again.

[dominikg]

export const get: RequestHandler = async (event) => {
if (event.request.method !== 'GET') {
return {};
}

might be safer to

export const get: RequestHandler = async (event) => {
  if (event.request.method !== 'GET' && event.request.method !== 'HEAD') {
    return {};
  }

automatic handling of HEAD requests does call the get handler and leaves out body in the end (otherwise we'd be unable to obtain the same headers as GET)

[Rich-Harris]

Closing as everything is working as intended — I agree that there's probably scope for more documentation around this, but that's the sort of thing that the tutorial will cover when we get round to writing it

[cdcarson]

Just a note about how this behavior is a subtle gotcha if you aren't aware of it and your handlers have side effects like setting cookies.

In my case it was setting a CSRF cookie (i.e. a plaintext token hashed with a secret.) If the post handler errored it created a new plaintext token to put in the form data and set the cookie. The get handler also set the cookie, but without updating the form data with the new token (since the form data already existed in the body.) So the CSRF check would pass if the form was submitted successfully the very first time, but fail subsequently.