arangojs icon indicating copy to clipboard operation
arangojs copied to clipboard

3xx http codes should be treated as errors

Open captain-refactor opened this issue 6 years ago • 4 comments

I found out, that during saving documents, the documents weren't saved, even no error was thrown.

Infrastructure:

Arango database is hosted behind the nginx load balancer. Nginx issues certificates and forces https by redirection.

Current behavior:

When arangojs receives 308 response, it doesn't throw an error, even the request hasn't been fulfilled. This behavior is really dangerous because it is easily missed.

Expected behavior

The client should throw an error.

captain-refactor avatar Jul 21 '19 20:07 captain-refactor

Shouldn't it actually follow the redirects?

pluma avatar Jul 23 '19 14:07 pluma

It should, but the error is better than nothing and it is easier to implement. The use case that someone wants to redirect database requests is improbable.

captain-refactor avatar Aug 04 '19 13:08 captain-refactor

So it should be implemented, or it should throw an error that it isn't supported.

captain-refactor avatar Aug 04 '19 13:08 captain-refactor

This depends. When a Foxx service responds with a redirect, that would likely not be an error but entirely expected. Throwing an error in that case by default would be bad.

In your case the proxy sending a redirect also isn't strictly an error and the expected behavior would probably be to follow that redirect in that case.

The correct approach IMO would be to add an option maxRedirects like request.js does (although it sets the default to 10 which may be unreasonably high for arangojs) and follow redirects by default. Hitting the limit would require throwing a specific error developers could recognise.

pluma avatar Aug 06 '19 15:08 pluma