Giter Club home page Giter Club logo

Comments (10)

adferrand avatar adferrand commented on May 28, 2024 1

Thanks a lot @e-gautier for you response. It clarifies the situation.

I think that changing from HTTP 200 to HTTP 204 in these endpoints makes sense. Alone this change would have had no impact, at least on Lexicon/Certbot side, and I bet in most other places also. Indeed the semantic usually applied is given by calling some abstraction method to check that the response is "valid", and any HTTP client will consider that 200 and 204 are valid. Here we are calling response.raise_for_status() and it follows this semantic.

The real problem is that these endpoints are now returning a body, which is an empty string. This is a direct violation of the intention of HTTP 204 status, and also of any response containing the header Content-Type: application/json since the body is not JSON parseable. You could argue here that no body is also problematic with this Content-Type, but I would say that it would be the case with any Content-Type, and also that not returning any body when there is no data to return is a sufficiently common approach in APIs to consider that any HTTP client implementation is very likely to treat correctly this case. For instance response.json() will not blow up and will just return None.

So I would strongly advise you to stop the API from returning a body when HTTP 204 is involved. Not only that it will kill right now all errors encountered by OVH users with Lexicon and Certbot, but you get a clean implementation that is future-proof.

from lexicon.

adferrand avatar adferrand commented on May 28, 2024

Why OVH, why ...

So basically the issue is due to an error raised by response.json() trying to extract an invalid JSON from the response body after a request to OVH APIs.

Three scenarios can happen with response.json():

  1. the response body is effectively a JSON and response.json() returns a JSON properly parsed as a dict,
  2. there is no response body and response.json() returns None (up to the caller to know that no response is expected and so no further processing is relevant),
  3. the response body exists and is not a JSON, and response.json() raises a requests.exceptions.JSONDecodeError.

Documentation of OVH has always stated that API response body was a JSON for some POST request, and no body for others (typically /refresh). So the code was designed to handle 1) and 2) since 3) should be considered as "something went wrong".

However it seems that now, OVH APIs may return an empty body instead of no body sometimes. I really mean sometimes here because the behavior appears to be flaky. I tested creation and deletion of records: sometimes no body is returned for /refresh for instance, and everything goes fine; sometimes an empty body is returned for the same endpoint (/refresh) and the code crashes.

I released a new version of Lexicon that explicitly catches 3) to return None instead of erroring out.

Release 3.15.1 is on its way.

from lexicon.

trnsnt avatar trnsnt commented on May 28, 2024

Hello,

Api has been changed to return a HTTP 204 instead of an HTTP 200 with body = null
I think, you should not try to call .json when status code is 204 as by definition HTTP 204 means no content !

from lexicon.

adferrand avatar adferrand commented on May 28, 2024

The code was already completely fine with either 200 or 204 code, and also fine with a response without body. In this case response.json() will return None.

The problem here was that some APIs started to return an empty body, for which response.json() cannot work given that an empty string is not a valid JSON.

What is worse is that the same endpoint returns sometimes no body, sometimes an empty body. I guess that it is related to some internal state on OVH side, like the endpoint triggers an action or not. But this is not specified anywhere in the doc, that is stated only that no body is returned for this kind of endpoints.

from lexicon.

camille-sound4 avatar camille-sound4 commented on May 28, 2024

Hello,

Thanks for the fast correction !
I had the same problem, and the correction worked for the Error adding TXT record with certbot 2.7.2 which is using lexicon 3.15.1.

However, I still have another error Zone mydomain.com is not deployed happening since last week, which is probably related :

Performing the following challenges:
dns-01 challenge for mysite.mydomain.com
Attempting to acquire lock 654219847864646 on /root/.lexicon_tld_set/publicsuffix.org-tlds/da645388e42c4fc4b1dfeeec3ac86334.tldextract.json.lock
Lock 654219847864646 acquired on /root/.lexicon_tld_set/publicsuffix.org-tlds/da645388e42c4fc4b1dfeeec3ac86334.tldextract.json.lock
Attempting to release lock 654219847864646 on /root/.lexicon_tld_set/publicsuffix.org-tlds/da645388e42c4fc4b1dfeeec3ac86334.tldextract.json.lock
Lock 654219847864646 released on /root/.lexicon_tld_set/publicsuffix.org-tlds/da645388e42c4fc4b1dfeeec3ac86334.tldextract.json.lock
Starting new HTTPS connection (1): eu.api.ovh.com:443
https://eu.api.ovh.com:443 "GET /1.0/auth/time HTTP/1.1" 200 10
https://eu.api.ovh.com:443 "GET /1.0/domain/zone/ HTTP/1.1" 200 None
https://eu.api.ovh.com:443 "GET /1.0/domain/zone/mydomain.com/status HTTP/1.1" 200 None
Encountered exception:
Traceback (most recent call last):
  File "/snap/certbot/3420/lib/python3.8/site-packages/certbot/plugins/dns_common_lexicon.py", line 246, in _resolve_domain
    with Client(self._build_lexicon_config(domain_name)):
  File "/snap/certbot-dns-ovh/current/lib/python3.8/site-packages/lexicon/client.py", line 151, in __enter__
    raise e
  File "/snap/certbot-dns-ovh/current/lib/python3.8/site-packages/lexicon/client.py", line 144, in __enter__
    provider.authenticate()
  File "/snap/certbot-dns-ovh/current/lib/python3.8/site-packages/lexicon/_private/providers/ovh.py", line 96, in authenticate
    raise AuthenticationError(f"Zone {domain} is not deployed")
lexicon.exceptions.AuthenticationError: Zone mydomain.com is not deployed

Thanks for your help

from lexicon.

e-gautier avatar e-gautier commented on May 28, 2024

Hi,

On behalf of the OVH DNS team I apologize for the trouble.
We migrated the domain API on another platform lately and this meant to be transparent (well...mostly).
We made the choice to switch the 200 statuses (body null) by 204 statuses (w/o content) and didn't expect this to have any impact.
The fickleness you encountered was mostly due to the API going through our blue/green migration process.
A communication will be made on the mailing list and the documentation will be updated as soon as possible.

from lexicon.

e-gautier avatar e-gautier commented on May 28, 2024

Hello @adferrand,
I agree about the Content-Type header.
What do you mean by these endpoints are now returning a body, which is an empty string? Afaik these endpoints are returning a new line right after the headers which is compliant to the RFC and the .json() method will always unconditionally try to parse JSON from the request content, empty or not.

from lexicon.

adferrand avatar adferrand commented on May 28, 2024

You are right and I interpretated wrongly the situation.

Before these APIs (and still a couple of others, like DELETE /domain/zone/{DOMAIN}/record/{RECORD}, were returning HTTP 200 with a body composed of null + carriage return, that JSON parsers interpret as the "null" value as appropriate for the target language (so None in Python) like the built-in module json (reference: https://docs.python.org/3/library/json.html#encoders-and-decoders), consumed by request to process JSON bodies.

In that extent HTTP 204 makes an actual body not necessary, even null. RFC is stating to just return a new line in the HTTP package, as these APIs do now. This is the unfortunate consequence in HTTP APIs of trying to express a null value with a text-based protocol, and here it implies to move from an actual body to no body, and this is not backward compatible.

So I concur with this change in the long term, but sadly it still represents a move in the API in short term that would deserve at least a migration path and potentially some versioning on the API...

from lexicon.

dauphinpasdroit avatar dauphinpasdroit commented on May 28, 2024

Hello guys,

Does the issue has been fixed lately ?

from lexicon.

arcameca avatar arcameca commented on May 28, 2024

Yes, it has been fixed.

from lexicon.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.