Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

3xx http codes should be treated as errors #613

Open
captain-refactor opened this issue Jul 21, 2019 · 4 comments
Open

3xx http codes should be treated as errors #613

captain-refactor opened this issue Jul 21, 2019 · 4 comments
Labels
Feature Request Request for new functionality to be added to the driver. semver-minor This issue does not require any backwards-incompatible changes to address.

Comments

@captain-refactor
Copy link

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.

@pluma
Copy link
Contributor

pluma commented Jul 23, 2019

Shouldn't it actually follow the redirects?

@captain-refactor
Copy link
Author

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
Copy link
Author

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

@pluma
Copy link
Contributor

pluma commented Aug 6, 2019

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 pluma added Feature Request Request for new functionality to be added to the driver. semver-minor This issue does not require any backwards-incompatible changes to address. labels Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request Request for new functionality to be added to the driver. semver-minor This issue does not require any backwards-incompatible changes to address.
Projects
None yet
Development

No branches or pull requests

2 participants