fix: TypeError: headers is not iterable

[mjad218]

At runtime headers may not be an instanceof Headers.

This causes a lot of bugs.

the function may get a js object as an argument.

Instead of looping directly through headers, I get the entries then loop through them.

[yusukebe]

Hi @mjad218

Thank you for the PR. Can you run yarn lint && yarn format to format the code?

[mjad218]

Sure, will push a commit soon

[yusukebe]

LGTM!

[yusukebe]

@mjad218

Thanks! Merging.

[usualoma]

Hi, @mjad218, Thanks for creating the pull request.
cc: @yusukebe

I'm sorry, but I didn't understand the need for this change. At least inside "node-server," I don't think anything other than "Headers" is passed as an argument. Does this mean it is used by directly importing it from the application?

I'd like to know the specific use case.

Also, when merging, we may need to add test code.

[mjad218]

@usualoma
I am using a Hone NestjsAdapter. The adapter actually does not call buildOutgoingHttpHeaders though.

[mjad218]

This is the adapter I am using
https://github.com/kiyasov/platform-hono/blob/main/src/adapters/hono-adapter.ts

[mjad218]

I believe what you are saying is correct given that it is called with valid arguments and you validate the headers object.

I reviewed its usage in the package here, I noticed whenever it's used, it has an if (headers instanceof Headers) condition before it. line 49

There was one place in the package that did not have this condition. line 90

Moving the validation inside the function itself is good; at runtime, you don't know what type of arguments it will get. (TS does not guarantee runtime checks)

[usualoma]

@mjad218 Thanks for the reply!

In the following line, we are referencing the headers property of the Response object so we can treat it as a Headers object without checking.

node-server/src/listener.ts

Line 90 in e63a808

const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers)

https://developer.mozilla.org/en-US/docs/Web/API/Response/headers

It is possible to break this convention in an application, but that is its responsibility, and it is not something that the node-server should handle.

Also, I don't think any Hono users have written code that returns a Request object that returns something other than a Headers object here.

I think that I don't need these changes.

[mjad218]

@usualoma I am using Nest.js and trying to get it working with Hono. And it seems that buildOutgoingHttpHeaders gets called with undefined.

I spent the past few days walking through Hono source code and I like how it's written and structured.

It is possible to break this convention in an application, but that is its responsibility, and it is not something that the node-server should handle.

I think this is the case, Nest.js is not designed to work with Hono. I am just trying to build an adapter for it.

It works best with Express and Fastify.

[usualoma]

@mjad218
Thanks for the explanation!
Sorry, I finally understood.
If that's the case, I see. We'll have to go with this approach.

[mjad218]

Great, Glad to hear that

[yusukebe]

@usualoma @mjad218

Thanks!

Also, when merging, we may need to add test code.

I'm sorry, but as @usualoma said, we have to add the test code. I'll add it myself tomorrow and take a test coverage for better quality.

[mjad218]

Sure, that's fine