Skip to content

Conversation

@jpulec
Copy link
Contributor

@jpulec jpulec commented Apr 10, 2025

It looks like during a refactor this logic got changed incorrectly. You need to await the promise in this catch block. Otherwise any error thrown inside responseViaResponseObject won't get caught and will bubble out the top.

It looks like during a refactor this logic got changed incorrectly. You need to await the promise in this catch block. Otherwise any error thrown inside `responseViaResponseObject` won't get caught and will bubble out the top.
@usualoma
Copy link
Member

Hi @jpulec. Thank you for creating a pull request.

I see. There should be an await to supplement asynchronous processing errors. It would be great if you could add the test code. Is that possible?

@jpulec
Copy link
Contributor Author

jpulec commented Apr 10, 2025

There's a minimal test, based on the issue I was experiencing. Best I can tell, if your fetchCallback returns an object that might be kinda like a response, i.e. has a body property, but doesn't implement .arrayBuffer() for example. it would cause an unhandled promise to be thrown. This patch should resolve that.

@usualoma
Copy link
Member

Hi @jpulec. Thank you. I think it's good!

Please execute the following command.

$ npm run format:fix && npm run lint:fix

@yusukebe yusukebe changed the title Handle Response Errors Correctly fix: Handle Response Errors Correctly Apr 11, 2025
Copy link
Member

@yusukebe yusukebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@yusukebe
Copy link
Member

@jpulec

Thanks! Looks good. I'll merge it and release a new version.

@yusukebe yusukebe merged commit 281a85f into honojs:main Apr 11, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants