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

Strict mode flag to return a non-zero exit code on warnings #2003

Closed
nguillaumin opened this issue Jan 19, 2017 · 12 comments
Closed

Strict mode flag to return a non-zero exit code on warnings #2003

nguillaumin opened this issue Jan 19, 2017 · 12 comments
Assignees
Milestone

Comments

@nguillaumin
Copy link

Hi,

Unless I'm missing something, AsciiDoctor will exit with a zero exit code when there are warnings (such as include files not being found, or images not being found).

It may be useful to have some sort of "strict" flag to return a non-zero exit code, especially in the context of continuous integration systems like Travis CI, Jenkins or GitLab CI. Without this it's impossible to detect errors in documents when they are automatically generated, and fail the build accordingly.

@bk2204
Copy link
Contributor

bk2204 commented Jun 11, 2017

This would be really useful for Git as well. I've looked into what this would entail, and it would probably involve wiring up warn (and maybe an error method as well) so that we could determine if warnings or errors had occurred and exit appropriately.

It's not entirely trivial, but it's possible with some refactoring.

@mojavelinux
Copy link
Member

You're correct that the Asciidoctor command reports a successful exit status even when there are warnings. The reason this was done was to be compliant with AsciiDoc Python. But I recognize that's nothing more than history. We should do the right thing.

The first thing that has to be done to accomplish this is to actually track warnings. Right now, we don't have any record of what got reported when the program is exiting (again, due to mimicking the behavior of AsciiDoc Python). However, we plan to do this in the 1.5.7 release. See #44. Then, we can change the exit code based on what got reported. I'll group these issues together.

@mojavelinux mojavelinux self-assigned this Jun 11, 2017
@mojavelinux mojavelinux added this to the v1.5.7 milestone Jun 11, 2017
@mojavelinux
Copy link
Member

The open status of #44 is biting me too.

@mojavelinux
Copy link
Member

Now that #44 is resolved, we can proceed with this issue.

What I propose is a new CLI option that can be used to specify a failure level:

--failure-level=WARN
--failure-level=ERROR

(the default is FATAL, which is consistent with the current behavior)

If this level is reached by a log message, it causes the CLI to return a non-zero exit code. You can even hide the messages using the -q flag and it will still work.

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Apr 16, 2018
…LI option

- add option to CLI to specify a failure level
- if this level is reached by a log message, it triggers a non-zero exit code
@mojavelinux
Copy link
Member

I decided on an enum option since failure is not binary. If you know of another application that offers something similar, but has a better name, I'd be happy to hear about it.

The other problem with the option name strict is that it's possibly misleading. It could mean that the document is being parsed in a strict mode, which is not the case here. I think we should focus on the level/severity/threshold.

@elextr
Copy link

elextr commented Apr 16, 2018

For the things I know, errors also return a non-zero status, but then for those apps errors are things that prevent a usable output, maybe Asciidoctor errors still produce an output?

Anyway you can define the levels as you want, so long as they can be turned into failures (non-zero status return).

@mojavelinux
Copy link
Member

mojavelinux commented Apr 16, 2018

In Asciidoctor, fatal errors don't produce output (which you see reported with the label FAILED). Errors (one level below) still produce output. Warnings (one level below errors) also produce output.

The understanding is that an error is probably going to lead to a failure down the line, but Asciidoctor is still able to produce output (for example, it violates the DocBook schema).

What we're doing here is altering the meaning of the severity so that you can get a non-zero exit code when a warning or error message is logged.

What we're still missing is a fail fast mode, which would halt the processor at the point of the error (or at the end of the document if you are processing multiple documents). That's a separate issue, but certainly possible within this new framework.

@elextr
Copy link

elextr commented Apr 16, 2018

The understanding is that an error is probably going to lead to a failure down the line, but Asciidoctor is still able to produce output (for example, it violates the DocBook schema).

Ok, makes sense.

What we're doing here is altering the meaning of the severity so that you can get a non-zero exit code when a warning or error message is logged.

Exactly right, then build tools can fail on the status return.

Asciidoctor is fast enough that unless you are Roger Penrose writing "Road to Reality" (1100 pages with much math and diagrams) then you probably won't save much with a fast fail.

mojavelinux added a commit to mojavelinux/asciidoctor that referenced this issue Apr 20, 2018
…LI option

- add option to CLI to specify a failure level (--failure-level=LEVEL)
- if this level is reached by a log message, it triggers a non-zero exit code
@mojavelinux mojavelinux removed the wip label Apr 20, 2018
@nguillaumin
Copy link
Author

Thanks guys!

@mojavelinux
Copy link
Member

🍻

@hho
Copy link

hho commented Jun 8, 2018

While it's awesome that we have that feature now, it doesn't seem to work correctly: When referencing a nonexistent image (raises a WARNING) with asciidoctor-pdf --failure-level=WARN my.adoc, the warning is printed, but the exit code remains 0.

@mojavelinux
Copy link
Member

That's because this is not yet supported in Asciidoctor PDF. Asciidoctor PDF does not yet use the logger. Until it does, the warnings are going to the wrong channel once you enter that converter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants