Parsing an inline link causes processor to crash for certain matches

[rwinch]

When using asciidoctor.js 2.2.7

I create an (invalid) index.adoc file as show below

= Heading

<<https://example.com,Example Link>>

I get the following error:

./node_modules/asciidoctor/bin/asciidoctor --version              
Asciidoctor.js 2.2.7 (Asciidoctor 2.0.22) [https://asciidoctor.org]
Runtime Environment (node v18.13.0 on linux)
CLI version 3.5.0
./node_modules/asciidoctor/bin/asciidoctor index.adoc --stacktrace

/home/rwinch/code/rwinch/asciidoctor-error/node_modules/asciidoctor-opal-runtime/src/opal.js:2476
        return $$($nesting, 'TypeError').$new("" + "no implicit conversion of " + (object.$class()) + " into " + (type))
                                         ^
constructor [TypeError]: no implicit conversion of NilClass into String
    at Function.$$type_error (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/asciidoctor-opal-runtime/src/opal.js:2476:42)
    at Function.$$coerce_to (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/asciidoctor-opal-runtime/src/opal.js:2487:26)
    at String.$String_$plus$7 (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/asciidoctor-opal-runtime/src/opal.js:6516:36)
    at $rb_plus (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:3226:90)
    at $$34 (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:3884:24)
    at String.$$gsub (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/asciidoctor-opal-runtime/src/opal.js:7048:26)
    at Opal.send (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/asciidoctor-opal-runtime/src/opal.js:1671:19)
    at constructor.$$sub_macros (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:3876:18)
    at /home/rwinch/code/rwinch/asciidoctor-error/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:3321:66
    at $$2 (/home/rwinch/code/rwinch/asciidoctor-error/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:3329:91)

Node.js v18.13.0

The latest 3.x asciidoctor.js and asdiidoctor (Ruby) do not fail.

I first noticed this problem when using Antora (which uses asdiidoctor.js 2.x line) in Spring Batch (see the build). It took a lot of effort to track this down. Generally, the failure and error message are very difficult to debug. It would be nice if this were handled in a way that made it easier to solve.

[mojavelinux]

This is the same issue as asciidoctor/asciidoctor#4570 and has been fixed in the upstream.

The xref shorthand you showed is not technically valid. That's requesting that the target of an internal xref be a URL, which doesn't make any sense. I'm curious where you are getting this syntax from.

[mojavelinux]

the failure and error message are very difficult to debug.

It was a completely unexpected error, so the code had no way to anticipate it and log a better message. It was an unintended side effect of fixing another issue. We try to make clear error messages when we can anticipate the fault.

[ggrossetie]

I believe this issue has been fixed upstream: asciidoctor/asciidoctor#4570

We need to release a new Asciidoctor.js version based on Asciidoctor (Ruby) 2.0.28.

This error in Java would be a NullPointerException and since the JavaScript code is transpiled from Ruby it does not make it easier to decipher.
Having said that, Asciidoctor maintainers can relatively easily find the root cause with this stacktrace (and fix the underlying bug).

[mojavelinux]

2.0.28

2.0.23

[ggrossetie]

Yep, 2.0.23! Sorry I'm on mobile and I mixed up the numbers 🫠

[rwinch]

Thank you for the quick responses!

The xref shorthand you showed is not technically valid. That's requesting that the target of an internal xref be a URL, which doesn't make any sense.

Thank you. I understand it is an invalid syntax. The difficulty is that the error message provides no clue as to what happened (more on this later).

I'm curious where you are getting this syntax from.

The error happened in May of 2022 as a part of a contribution. My best guess is that it was just an honest mistake and wasn't gotten from anywhere in particular.

It was a completely unexpected error, so the code had no way to anticipate it and log a better message. It was an unintended side effect of fixing another issue. We try to make clear error messages when we can anticipate the fault.

I think that for unexpected error, the message is fine. However, I think that it would be useful to provide more context to troubleshoot unexpected errors. Perhaps this should be another ticket(s) and possibly in Asciidoctor and/or Antora, but I wonder:

• Is it possible to include the file name and line number that caused an unexpected error?
• For Antora, would it be possible to include the Git Repository and ref that the error happened in when unexpected errors (like this) happen?

Having this information would have made this much easier to figure out what was happening.

A little context for how the error was encountered might be helpful. In this particular case, Spring Batch had not made any changes to the documentation or its docs build, so it was surprising when the error started. This also made it difficult to figure out what was causing the error because there wasn't a diff in spring batch that could be used to help isolate the problem.

I'd like to start by stating that I'm very novice with JavaScript, so my assessment was a bit difficult to do, but also may be incorrect. I believe that the error started because Antora brings in asciidoctor ~2.2 which means that when asciidoctor.js 2.2.7 was released in March, the Spring Batch build started using asciidoctor.js 2.2.7 and the build started failing despite having worked for months with the same build and docs. If this is true, I have a few questions:

1. Should specific versions be used for the js dependencies? I'm very new to JS, but it seems that non-specific versions are pretty common in JS so perhaps this is not expected for the JS ecosystem.
2. If the version ranges are not specific, then perhaps the Spring Team should be using package-lock.json to ensure that our build is repeatable. If that is the case, I wonder if it would be valuable to have written guidance on this (especially in the Maven/Gradle plugins since Java devs are use to specific versions being used).

[mojavelinux]

The error happened in May of 2022 as a part of a contribution. My best guess is that it was just an honest mistake and wasn't gotten from anywhere in particular.

Thanks for clarifying. I was just asking in case we had some stray documentation somewhere that I needed to track down.

I think that it would be useful to provide more context to troubleshoot unexpected errors.

Again, it should never have crashed that way it did. It was completely unexpected and therefore not possible to handle.

We already do all the things you are suggesting when we can anticipate an error. But the code actually has to make it to the error handler. In this case, it just didn't make it that far and crashed earlier. There's nothing we can do when the program fails in this way because the runtime stops.

I think you are harping on a very unfortunate situation. This should never have happened in a patch release. But it did. And we did our best to address it as quickly as we could in Asciidoctor core. Given the syntax was so rare, I did not anticipate it affecting Antora. But it did. So now another release (this time of Asciidoctor.js) is warranted.

We have a ridiculous number of tests and, in this case, it still slipped through. More tests are always needed, so it just emphasizes why we need to keep prioritizing them.

I don't really find further discussion on this issue to be productive. We know what happened and we fixed it. The next step is to push out an Asciidoctor.js release to pick up the upstream fix so that Antora benefits from it as well.

[mojavelinux]

I'm very new to JS, but it seems that non-specific versions are pretty common in JS so perhaps this is not expected for the JS ecosystem.

We intentionally removed the exact dependency on Asciidoctor.js in Antora and switched it to the latest of the preferred minor (2.2). We made an assumption that that time that a patch release should never introduce a problem and allows Antora to benefit from the patch update immediately without a new release. Due to a very unlucky situation, it did (but still limited to invalid and very esoteric AsciiDoc syntax). I'm not going to change the policy in Antora because of what happened. I still think my reasoning for using ~2.2 is sound. It's extremely unlikely this would happen again.

[mojavelinux]

I did some more thinking about this and I've come to realize that there is more we can do in Antora to improve the log message when a situation like this occurs. I've opened the following issue to track this improvement: https://gitlab.com/antora/antora/-/issues/1123 Let's continue the discussion there as it pertains to Antora.

[ggrossetie]

Asciidoctor.js 2.2.8 is out, could you please try again?

[mojavelinux]

I verified the fix and added a test to Antora for it.