mikitex70 / plantuml-markdown

PlantUML plugin for Python-Markdown
BSD 2-Clause "Simplified" License
192 stars 55 forks source link

WARNING does not fail if in strict mode #80

Closed anb0s closed 1 year ago

anb0s commented 1 year ago

The WARNING does not fail if in strict mode and therefore we have broken images in our docs, e.g. we see this log entries

WARNING in "uml" directive: remote server has returned error 400
WARNING in "uml" directive: remote server has returned error 429
anb0s commented 1 year ago

sorry i'm using different versions, in 3.7.1 i see an exception and build fails:

WARNING in "uml" directive: remote server has returned error 429 on GET: {
  "message":"API rate limit exceeded"
}
ERROR    -  Error reading page 'Frame/DomainModel/s0_OIE_StartUp.md': 'NoneType' object has no attribute 'decode'

but it would be good to have build continuing and report WARNING and fail the build if strict mode...

nejch commented 1 year ago

It might be good to add context here:

This is happening inside mkdocs (admittedly probably the main consumer of this extension). For mkdocs to pick up logged warnings in its --strict mode, loggers actually have to be called mkdocs.some_logger, and this is more used in mkdocs plugins, not generic libraries like this one.

So I'm not sure if this is in the scope of this project, but @mikitex70 can say more on that.

Context on how loggers can be used in mkdocs strict mode: https://github.com/mkdocs/mkdocs/issues/2082


I think the issue you're seeing in the exception might be that _render_diagram() returns None for diagram when there is an error, and it tries to later decode it: https://github.com/mikitex70/plantuml-markdown/blob/master/plantuml_markdown.py#L190

But i haven't looked at it more in-depth. It might be a real issue though, unrelated to the logging topic.

mikitex70 commented 1 year ago

I think the error raises from line 412: if a diagram is incorrect the PlantUML server returns an HTTP 400 error with the error image as the body. The lines tries to decode it as an UTF-8 string and raises an exception because of an invalid encoding. I've found this problem while implementing #76, and it is fixed in the develop branch. The rate limit (HTTP 429) should be fixed in the 3.7.2 version. I will check the code to make sure that every error is logged and does not generate uncontrolled exceptions.

anb0s commented 1 year ago

Great! I can confirm that rate limit (HTTP 429) is now fixed with 3.7.2 😄

mikitex70 commented 1 year ago

Closing this issue as it seems resolved. Thanks for reporting it.