Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug?]: onBeforeResponse response object is empty in middleware #1278

Closed
2 tasks done
sabercoy opened this issue Jan 23, 2024 · 4 comments
Closed
2 tasks done

[Bug?]: onBeforeResponse response object is empty in middleware #1278

sabercoy opened this issue Jan 23, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@sabercoy
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

The response object is not properly populated according to the Response TypeScript definition. There is only a "body" property. I am attempting to write headers.

import { createMiddleware } from '@solidjs/start/server'

export default createMiddleware({
  onBeforeResponse(event, response) {
    console.log(response)
    response.headers.set('test', 'test')
  }
})
{
  body: ReadableStream { locked: false, state: 'readable', supportsBYOB: false }
}
[nitro] [request error] [unhandled] Cannot read properties of undefined (reading 'set')
  at onBeforeResponse (./.output/server/chunks/nitro/node-server.mjs:7610:29)  
  at ./.output/server/chunks/nitro/node-server.mjs:7599:31  
  at _callHandler (./.output/server/chunks/nitro/node-server.mjs:2281:13)  
  at process.processTicksAndRejections (node:internal/process/task_queues:95:5)  
  at async Object.handler (./.output/server/chunks/nitro/node-server.mjs:2390:19)  
  at async Server.toNodeHandle (./.output/server/chunks/nitro/node-server.mjs:2579:7)

Expected behavior 🤔

The response object, if provided inside onBeforeResponse, should be populated and usable.

Steps to reproduce 🕹

Steps:

  1. Init a solid start bare project
  2. create a middleware that uses onBeforeResponse and the response parameter

Context 🔦

I am trying to find a way to write response headers for the user, as well as access response headers for logging.

Your environment 🌎

"dependencies": {
    "@solidjs/start": "^0.4.10",
    "solid-js": "^1.8.11",
    "vinxi": "^0.1.4"
  }
@sabercoy sabercoy added the bug Something isn't working label Jan 23, 2024
@ryansolid
Copy link
Member

ryansolid commented Jan 23, 2024

My understanding is this by design of Nitro hooks. The body is whatever was returned. I'm thinking we should probably just wrap this differently and not use their conventions as it is odd. We must have typed things wrong.

Generally use setHeader rather than try to manipulate the response directly I think might be the coarse of action. This does deserve more discussion though.

@sabercoy
Copy link
Author

The body is whatever was returned.

Yeah after some further digging this does seem to be the behavior. I agree its odd. Coming from other API's I am used to set the headers on the response, I should have realized there was a solid-start provided API to do that. Which now makes me think of what is the purpose of the provided Nitro response. I guess it is useful if someone really needs to dig into something. But if its kept, then yeah should at least have some more accurate typings.

@ryansolid
Copy link
Member

It seems the design was intentional so people could swap the response, but we could just change the API here to suit our needs I think. I'm open to discussion on that. The reason I want the response there is so I can look at it mostly. I suppose if what I got was an Error I might want to send back a different response. I should see how it cascades through. Part of this comes from better understanding the current behavior too.

@ryansolid
Copy link
Member

So I'm going to fix the types for this issue. What it should be doing is a more open discussion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants