elysia icon indicating copy to clipboard operation
elysia copied to clipboard

Empty body in POST request causing silent errors when reading from context parameter

Open JohnRoseDev opened this issue 11 months ago • 4 comments

What version of Elysia.JS is running?

1.0.0

What platform is your computer?

Darwin 22.5.0 x86_64 i386

What steps can reproduce the bug?

After upgrading from 0.8.17 to 1.0.0 I noticed odd behavior today with some of my post requests. I was eventually able to narrow it down to the fact that it happens:

  1. when the client makes a POST request without the body attribute of the RequestInit config being set as part of the browser fetch (if I set body to {}, everything works fine)
  2. when I read from the request or body object of the ctx parameter within the route handler in elysia, as odd as this seems (Both of these conditions need to happen)

Example:

const app = new Elysia()
  .use(cors({
    origin: 'localhost:3000',
    allowedHeaders: ['Content-Type', 'x-csrf-token'],
  }));

const someFunction = (request?: Request) => {
  return true;
};

app.post('/test', async ({ request }) => {
  const value = someFunction(request);

  return new Response(JSON.stringify({ message: 'hi' }));
});

This errors out silently and elysia responds to the client with a 400.

Even just destructuring body from context causes a 400:

app.post('/test', async ({ body }) => {
  return new Response(JSON.stringify({ message: 'hi' }));
});

I do not understand how the mere destructuring can cause an error that otherwise does not appear (I guess it's related to the transpiled code).

The following works on the other hand, no 400 here:

app.post('/test', async ({}) => {
  return new Response(JSON.stringify({ message: 'hi' }));
});
// 200 is returned

Destructuring request works fine as well, contrary to body:

app.post('/test', async ({ request }) => {
  return new Response(JSON.stringify({ message: 'hi' }));
});
// 200 is returned

Only when I start to log request, it errors out again:

app.post('/test', async ({ request }) => {
  console.log(request);
  return new Response(JSON.stringify({ message: 'hi' }));
});
// 400 is returned

Additional information

  • The preview of the 400 response is "Failed%20to%20parse%20body%20as%20found%3A%20undefined"

  • I didn't test versions in between 0.8.17 and 1.0.0, I only tested all versions after 1.0.0 and they all have the same behavior. In 0.8.17 there are none of these errors even when the body attribute of the fetch request is not set.

JohnRoseDev avatar Mar 22 '24 00:03 JohnRoseDev

The following code is responsible for this behavior, including a change that was made a month ago:

hasBody is assumed to be true for every POST request: https://github.com/elysiajs/elysia/commit/711925a5b5561452b58979b26917cb13a6cf8d9c?diff=unified&w=0#diff-9043fd343dba2fc691e6d1577d67b4c164b7cdc2ce399d3a3cc787e06dd81c32R377 It seems to be a debated topic whether it's common/ok to send POST requests without body: https://stackoverflow.com/questions/4191593/is-it-considered-bad-practice-to-perform-http-post-without-entity-body

In any case, this change from a month ago is causing the (intentional) error if there is no body: https://github.com/elysiajs/elysia/commit/711925a5b5561452b58979b26917cb13a6cf8d9c?diff=unified&w=0#diff-9043fd343dba2fc691e6d1577d67b4c164b7cdc2ce399d3a3cc787e06dd81c32R603

JohnRoseDev avatar Mar 22 '24 11:03 JohnRoseDev

I faced with this and found it strange. Even though there is a debate about it, I believe it should be decided by the developer whether to throw an error or not.

If we have structured error messages, we have to check for ParseError in our error handler. It shouldn't be necessary as there is already a body validation happening by typebox.

I think in such cases body should be undefined instead of throwing error.

Using Elysia 1.1.5

ayZagen avatar Aug 07 '24 07:08 ayZagen

Hi, I want to follow up this bug.

I'm not sure if I understand the error correctly or if it was fixed on 1.1.x as there are several change that might have fix this unintentionally.

I tried with the following code, and everything seems to works fine:

import { Elysia, t } from 'Elysia'

const app = new Elysia()
	.post('/test', async ({ body }) => {
		return new Response(JSON.stringify({ message: 'hi' }))
	})
	.listen(3000)

fetch('http://localhost:3000/test', {
	method: 'POST'
})
	.then((x) => x.status)
	.then(console.log)

Can you give me some reproducible code like above so I can put it on unit test as well, thanks a lot

SaltyAom avatar Aug 27 '24 08:08 SaltyAom

Hi @SaltyAom, The error happens you send the request as json, not sure if this is considerable as a bug or intended functionality. (Elysia version 1.1.12)

83 | export class ParseError extends Error {
84 | 	code = 'PARSE'
85 | 	status = 400
86 |
87 | 	constructor() {
88 | 		super('Failed to parse body')
       ^
error: Failed to parse body
 code: "PARSE"

server:

.post('/test', async () => {
    return new Response(JSON.stringify({ message: 'hi' }));
})

client:

await fetch('https://localhost:3000/test', {
   method: 'POST',
   headers: {
     'Content-Type': 'application/json', // important
   },
});

If we remove the content-type header, then sending an empty body works and does not create an error. I guess this is expected behavior since sending an empty body does not make sense if you explicitly say that you're sending json.

Regarding the described strange intermittent behavior around whether or not the request or body object was destructured, this is no longer the case. The error always happens, regardless of whether or not we do something with the body param.

JohnRoseDev avatar Sep 09 '24 13:09 JohnRoseDev