c.json returning empty string for undefined values

[yusukebe]

Discussed in https://github.com/orgs/honojs/discussions/2338

^{Originally posted by **marcosrjjunior** March 11, 2024} I'm mainly creating this ticket to understand a bit more about why this is happening and potentially flag an issue. One of my functions was returning `undefined` instead of `false` and the `c.json` is forcing the response to be an "" (empty string) instead of throwing an error.  **Response:** "" --- Reading the [json implementation here](https://github.com/honojs/hono/blob/4ca5f60def726731b3afbf3f6137f0a4b92d8b27/src/context.ts#L469) (hopefully that is the correct place). I noticed that there possibly some extra things going on there: - `JSON.stringify(object)`. Why do we need to run throguh a JSON.stringify before returning? - Re-creating the response Because of the issues mentioned above I replaced it to use the main [response](https://developer.mozilla.org/en-US/docs/Web/API/Response/json_static) instead. 

[damianpumar]

Hello @yusukebe I think we can not replace JSON.stringify to Response.json, because the JSON with undefinedreturns undefined and Response throws an exception. So, my proposal is create a unit test for context.json function with undefined and fix this method.

What do you think ? Thanks! 

[yusukebe]

Hi @damianpumar

The interesting thing about this problem is that the result of new Response(undefined) is different at different runtimes. although it might be expected to throw an error. So I thought of using Response.json() to make c.json() behave according to each runtime.

But I'm still trying to figure out if that's the best option.

What do you think the result of c.json(undefined) should be?

[marcosrjjunior]

@damianpumar the question is, why do we need to extra process JSON.stringify if we know the response is already a JSON?

On the scenario described above the c.json is treating the undefined and returning an empty string ("") which forces the users to treat the client that is consuming the endpoint or fix the function to return false instead. (more examples on the discussion mentioned above).

again, I don't see any value of using c.json on that scenario, if I can direct call Response.json and get the best out of if.

---

• maybe we can also mention that on the documentation somewhere, I'm sure some people are aware, but some don't know they can call Response or Request direct, which is great.

A few ideas, extracted from nextjs docs.

@yusukebe

[damianpumar]

Thanks guys for your explanation about this!, I didn't know that each runtime has differents results. From my point of view, we can standarise this behaviour in hono, throwing an error, each developer must be responsible to return a valid json in this response otherwise should handle this case. In summary, I think we can return the same result for all runtimes. What do you think?

[yusukebe]

Hi @marcosrjjunior @damianpumar !

As @damianpumar says, validating the value passed to c.json() on the Hono side is one good way. However, I am concerned about the performance degradation and code increase that validation would cause. So I think if Response.json() solves the problem, it will be fine with that.

This is an important issue. Give me a little more time to think!

[damianpumar]

Hi @yusukebe, referencing your message here-> https://github.com/orgs/honojs/discussions/2338#discussioncomment-8739141 I tried the same code with node (throw error) bun ("") and cloudflare ("undefined"), and I think we can homogenize this behaviour with little code by making sure that the c.json argument is not undefined, otherwise throw the error.

Just let me know and I will make this change!

Thanks guys!

[yusukebe]

Ah! Maybe we can refer to the implementation by Deno:

https://github.com/denoland/deno/blob/0d43a63636c97886c10c6b8ce05fdb67cd2d8b91/ext/web/00_infra.js#L382-L388

function serializeJSValueToJSONString(value) {
  const result = JSONStringify(value);
  if (result === undefined) {
    throw new TypeError("Value is not JSON serializable.");
  }
  return result;
}

And we have to consider that it's possible to be a breaking change.

[marcosrjjunior]

if (response === undefined) {
  throw new TypeError('Value is not JSON serializable.')
}

return Response.json(response)

I think the stringify function doesn't need to be in that.

that worked fine when running using bun and node.

[yusukebe]

@marcosrjjunior

But then, I think we don't need to use Response.json().

[marcosrjjunior]

ah yes, that make sense..

I'm curious if there is a significant difference between the Response.json() and JSON.stringify, hopefully not.

[yusukebe]

If we follow Deno's implementation, the difference is whether or not it throws an error when the value is undefined. That would be nice. But I'm not sure if I should fix it right away; some apps that are currently working may not work.

function serializeJSValueToJSONString(value) {
  const result = JSONStringify(value);
  if (result === undefined) {
    throw new Error("Value is not JSON serializable."); // <===
  }
  return result;
}