asciidoctor-kroki breaks when using remote content repositories #416

joostschriek · 2023-06-16T13:45:42Z

we have various playbooks and remote content sources setup. we've recently introduced kroki as a way to not manually copy paste diagrams from MSPaint or lucid w/e. While this all works perfectly with local content sources, having them as remote content sources results in the following error;

Skipping diagramsnet block macro. The "path" argument must be of type string. Received undefined

During our builds we install all the required npm extensions;

npm i -g @antora/cli@3.0.0 @antora/site-generator@3.0.0 asciidoctor-kroki asciidoctor-external-callout

we call the extensions like so

.Context view diagram
diagramsnet::{partial-prefix}/context-view.drawio[format=png]

During our debugging we've figured out that local works but remote doesn't. We've tried the following remediations;

setting the cache dir to see if we can look at the content it loads. this sadly didn't work as the only thing in the cache dir are the ui-bundle and the bare repos.
setting the kroki attributes on the module.yaml (yes, they have always been in the playbook). this also didn't work.
switching version. we're currently on 3.0, either down- or upgrading didn't work.
various different incantations of the diagramsnet block.

A remote playbook

site:
  title: SuperSecure Internal Documentation
  url: https://supersecure.acme.dope
  start_page: documentation-solution-overview::introduction-to-docs-as-code.adoc

content:
  sources:
 	.....more content sources
    - url: https://github.com/acme-dope/cloudops-docs.git
      branches: main
      start_path: documentation
   
asciidoc:
  attributes:
    kroki-server-url: https://kroki.acme.dope
    kroki-fetch-diagram: true
  extensions:
    - asciidoctor-kroki
    - asciidoctor-external-callout

	...ui bundle

A local playbook

site:
  title: Cloud Operations Documentation
  url: https://supersecure.acme.dope
  start_page: cloud-operations-documentation::index.adoc
content:
  sources:
    - url: ./../../
      start_path: documentation

asciidoc:
  attributes:
    kroki-server-url: https://kroki.acme.dope
    kroki-fetch-diagram: true
  extensions:
  - asciidoctor-kroki
  - asciidoctor-external-callout

  ...ui bundle

The text was updated successfully, but these errors were encountered:

ggrossetie · 2023-06-17T23:05:26Z

Could you please tell me what is the value of {partial-prefix}?

ggrossetie · 2023-06-18T12:19:45Z

The "path" argument must be of type string. Received undefined

This error usually comes from Node.js when using path.dirname(path). Since we don't print the complete stacktrace it's hard to know where exactly this error comes from.

@mojavelinux Is it possible to retrieve the --stacktrace flag from an Asciidoctor extension used in an Antora context? That way, I could print the stacktrace to the console if the flag is present.
Or maybe it's fine to use the Asciidoctor logger?

mojavelinux · 2023-06-18T21:14:54Z

Before I can investigate, I need a reproducible test case or example to work with. https://github.com/acme-dope/cloudops-docs.git is not a real git repository, so there's no way for me to test this right now. I'm not going to spend the time to set up the scenario on my own since a) I may not reproduce the scenario correctly and b) it's not fair for me to have to do that.

mojavelinux · 2023-06-18T21:16:32Z

If --stacktrace is specified, we would expect to see a stacktrace when something fails. If it's absent, it could be related to this (now fixed) issue https://gitlab.com/antora/antora/-/issues/1049.

ggrossetie · 2023-06-18T21:22:45Z

If --stacktrace is specified, we would expect to see a stacktrace when something fails. If it's absent, it could be related to this (now fixed) issue https://gitlab.com/antora/antora/-/issues/1049.

The error is captured by the Asciidoctor Kroki extension. My question was, can the extension be aware of the --stacktrace flag? If so, I can do:

try {
  // code...
} catch (err) {
  logger.warn('Something went wrong, ignoring...')
  if (stacktrace) {
    console.trace()
  }
}

mojavelinux · 2023-06-18T21:38:23Z

Ah, I see. Good question.

The stacktrace option specified on the commandline is not directly accessible from an Asciidoctor extension*. See https://gitlab.com/antora/antora/-/blob/main/packages/cli/lib/cli.js#L116. You could, of course, look for it in the process args. But I don't recommend doing that. Instead, this error should either be logged or thrown. And when it is logged, the stack should be preserved. That way, it falls through to Antora's CLI or logger to be reported. Extensions should not try to process or swallow this information (unless the message contains all the necessary information). In short, the information should bubble up.

Here's how we preserve the stack inside of Antora: https://gitlab.com/antora/antora/-/blob/main/packages/content-aggregator/lib/aggregate-content.js#L987-1017

Here's how we log an error in an extension: https://gitlab.com/antora/antora-assembler/-/blob/main/packages/assembler/lib/asciidoctor/reducer-extension.js#L181 (however, the error object should be logged instead of just the message).

(*) I do wonder if we should transfer this option to a key in the playbook's runtime category. Hmmm.

joostschriek · 2023-06-19T19:09:19Z

Could you please tell me what is the value of {partial-prefix}?

@ggrossetie partial-prefix is a variable that points to the correct directory in the partial folder of antora. for this case it's not really relevant, but i've included it in the example

The "path" argument must be of type string. Received undefined

This error usually comes from Node.js when using path.dirname(path). Since we don't print the complete stacktrace it's hard to know where exactly this error comes from.

@ggrossetie Yes. Very hard and non-trivial :) fwiw during setting up the test case is verified (once again) that it's the remote content-source causing the problem, and that we don't hit the kroki-server (or parse the value) before we get the error. I'm not familiar with the pipeline, but maybe that bit of information is useful to you.

Before I can investigate, I need a reproducible test case or example to work with. https://github.com/acme-dope/cloudops-docs.git is not a real git repository, so there's no way for me to test this right now. I'm not going to spend the time to set up the scenario on my own since a) I may not reproduce the scenario correctly and b) it's not fair for me to have to do that.

@mojavelinux @ggrossetie Sure, I followed the contributing guidelines that I was able to find. Not everybody wants a complete repro. I put it up @ joostschriek/antora-kroki-problem. The reproduction can be done by commenting out the remote-content-source for the local-content-source.

⚠️ note i cannot share the kroki server url that we use, as it's not accessible from the outside. there are ample instruction on how to quickly set this up with docker or k8s @ how to install kroki. I did find this compose file that i once used, but i've recently moved them all into a corp k8s cluster.

some more bash and debug outputs

what the site looks like with the remote-content-source

The remote-content-source run

joost at acidburn in ~/src/joostschriek/kroki-antora-problem on master [!]
$ antora --stacktrace playbook.yml 
[clone] https://github.com/joostschriek/kroki-antora-problem.git [###################################################################]
Skipping diagramsnet block macro. The "path" argument must be of type string. Received undefined
Skipping diagramsnet block macro. The "path" argument must be of type string. Received undefined
Site generation complete!
Open file:///home/joost/src/joostschriek/kroki-antora-problem/build/site in a browser to view your site.

and the file tree looks like this

the local-content-source run

joost at acidburn in ~/src/joostschriek/kroki-antora-problem on master [!]
$ antora --stacktrace playbook.yml 
Site generation complete!
Open file:///home/joost/src/joostschriek/kroki-antora-problem/build/site in a browser to view your site.

and the filetree looks like this

ggrossetie · 2023-06-20T12:00:42Z

The complete stacktrace:

error TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:387:5)
    at validateString (node:internal/validators:162:11)
    at Object.dirname (node:path:1276:5)
    at Object.dirname (/home/guillaume/Workspace/opensource/kroki-antora-problem/node_modules/asciidoctor-kroki/src/antora-adapter.js:51:30)
    at constructor.<anonymous> (/home/guillaume/Workspace/opensource/kroki-antora-problem/node_modules/asciidoctor-kroki/src/asciidoctor-kroki.js:205:32)
    at Object.apply (/home/guillaume/.npm/_npx/0d33774fda1fdb40/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:26026:21)
    at constructor.$$instance_exec (/home/guillaume/.npm/_npx/0d33774fda1fdb40/node_modules/asciidoctor-opal-runtime/src/opal.js:3846:24)
    at Opal.send (/home/guillaume/.npm/_npx/0d33774fda1fdb40/node_modules/asciidoctor-opal-runtime/src/opal.js:1671:19)
    at Proxy.$$19 (/home/guillaume/.npm/_npx/0d33774fda1fdb40/node_modules/@asciidoctor/core/dist/node/asciidoctor.js:19823:24)
    at Opal.send (/home/guillaume/.npm/_npx/0d33774fda1fdb40/node_modules/asciidoctor-opal-runtime/src/opal.js:1671:19) {

And the cause is that target.src.abspath is undefined:

https://github.com/ggrossetie/asciidoctor-kroki/blob/e5e8e6aba7389535fbf022ee58fdfe4e0a91b9f0/src/antora-adapter.js#L51

We are using diagramDir to preprocess PlantUML and Vega-Lite but we should probably do things differently in Antora. I will do a quick fix for now.

ggrossetie · 2023-06-20T12:31:28Z

It definitely looks better using the Logger from the parent document (Asciidoctor):

const errWrapper = new Error(`Skipping ${diagramType} block.`)
errWrapper.stack += `\nCaused by: ${err.stack || 'unknown'}`
parent.getDocument().getLogger().warn({ err: errWrapper })

Current output

[14:22:09.449] WARN (asciidoctor):
    file: docs/modules/ROOT/pages/index.adoc
    source: https://github.com/joostschriek/kroki-antora-problem.git (branch: master | start path: docs)
    msg: {
      "err": {
      }
    }

Expected output

Would it be possible to use stdSerializers.err (from Pino) on msg.err ?

[14:22:09.449] WARN (asciidoctor): Skipping diagramsnet block
    file: docs/modules/ROOT/pages/index.adoc
    source: https://github.com/joostschriek/kroki-antora-problem.git (branch: master | start path: docs)
    msg: {
      err: {
        type: 'Error',
        message: 'Skipping diagramsnet block',
        stack: '...'
      }
    }

Edit: Ideally, something like that would be great:

parent.getDocument().getLogger().warn({ cause: err }, `Skipping ${diagramType} block.`)

joostschriek · 2023-06-20T15:07:09Z

And the cause is that target.src.abspath is undefined:

https://github.com/ggrossetie/asciidoctor-kroki/blob/e5e8e6aba7389535fbf022ee58fdfe4e0a91b9f0/src/antora-adapter.js#L51

We are using diagramDir to preprocess PlantUML and Vega-Lite but we should probably do things differently in Antora. I will do a quick fix for now.

i'm guessing that the antora (component of kroki?) with remote-content-sources returns relative paths, which is why this is error out?

mojavelinux · 2023-06-21T00:31:37Z

If Asciidoctor Kroki requires a file to be at an absolute path on the file system, it must write that file to disk and use that temporary file. Files in Antora are virtual / in-memory. Extensions should adhere to this contract. They should not behave differently if the file is sourced from the git tree instead of the worktree.

(I want to clarify that the distinction in Antora is not "remote content sources" vs "local content sources". Rather, it's whether a file is taken from a git tree (the git index) or it is taken from a worktree (a checked out copy of a ref, which equates to files on the local filesystem)).

ggrossetie · 2023-06-21T20:40:43Z

I think we need to rethink how to implement the preprocessor that resolves PlantUML !include directives and Vega datasets when running in an Antora context.

But before that I think we need to specify how it should work. Currently, we are trying to resolve them using Antora Resource IDs and/or using a relative paths. I'm not sure what is the best practice here.
If the PlantUML or Vega file is part of an Antora documentation component, I think it makes sense to use resource IDs rather than relative paths.

Anyway, until we specify how it should work, my quick fix is to use ospath.dirname(target.src.abspath || target.src.path).

Regarding the logger, is it possible to use a message AND a data object. If not I think I will use an err object and try to format it. I might also add the name of the extension to make it clear that the error comes from Asciidoctor Kroki.

I guess I could also add the source_location, if I can get access to the reader.

joostschriek · 2023-07-04T19:22:27Z

@ggrossetie i made an attempt at spot fixing this issue, i'm sure you'll let me know if everything's ok 😃

mojavelinux · 2023-07-05T06:48:07Z

I might also add the name of the extension to make it clear that the error comes from Asciidoctor Kroki.

We are going to establish a contract here in Antora. If you set the package property on the err object, that value will be used as the logger category when the error is logged at the fatal level. You can add that value in the message too if you like, but at least set the package property.

mojavelinux · 2023-07-05T06:50:39Z

we are trying to resolve them using Antora Resource IDs

This should work fine. However, you need to make sure that the code calls the write API. You can find an example in Antora's own include extension. See https://gitlab.com/antora/antora/-/blob/main/packages/asciidoc-loader/lib/include/resolve-include-file.js

When running an Antora, a portable extension should never rely on src.abspath. The value of this property is intended to be informational only, such as for use in a log message.

…ded to be informational (and can be undefined)

ggrossetie · 2023-07-08T13:52:07Z

@mojavelinux I get a TypeError: msg.$inspect is not a function when calling logger.warn with an object. I guess the reason is that I didn't create a proper Ruby/Opal object. I will try to attach an $inspect function to the object and call JSON.stringify.

We should _not_ rely on abspath since this property is intended to be informational (and can be undefined).

mojavelinux · 2023-07-08T19:45:31Z

The object has to be created with message_with_context. See https://gitlab.com/antora/antora/-/blob/main/packages/asciidoc-loader/lib/include/include-processor.js#L186-192

We should _not_ rely on abspath since this property is intended to be informational (and can be undefined).

joostschriek mentioned this issue Jul 4, 2023

Ensure that we can lookup the antora provided paths #419

Closed

ggrossetie added the 🐞 bug Something isn't working label Jul 8, 2023

ggrossetie added a commit that referenced this issue Jul 8, 2023

resolves #416 should not rely on abspath since this property is inten…

4edef30

…ded to be informational (and can be undefined)

ggrossetie mentioned this issue Jul 8, 2023

resolves #416 should not rely on abspath since this property is intended to be informational (and can be undefined) #422

Merged

ggrossetie added a commit that referenced this issue Jul 8, 2023

resolves #416 resolve Antora resource using resource ids

e7a1438

ggrossetie added a commit that referenced this issue Jul 8, 2023

resolves #416 resolve Antora resource using resource ids

304edf9

We should _not_ rely on abspath since this property is intended to be informational (and can be undefined).

ggrossetie added a commit that referenced this issue Jul 8, 2023

resolves #416 resolve Antora resource using resource ids

24eec00

We should _not_ rely on abspath since this property is intended to be informational (and can be undefined).

ggrossetie closed this as completed in #422 Jul 8, 2023

ggrossetie added a commit that referenced this issue Jul 8, 2023

resolves #416 resolve Antora resource using resource ids (#422)

f2e3836

We should _not_ rely on abspath since this property is intended to be informational (and can be undefined).

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

asciidoctor-kroki breaks when using remote content repositories #416

asciidoctor-kroki breaks when using remote content repositories #416

joostschriek commented Jun 16, 2023 •

edited

Loading

ggrossetie commented Jun 17, 2023

ggrossetie commented Jun 18, 2023

mojavelinux commented Jun 18, 2023

mojavelinux commented Jun 18, 2023

ggrossetie commented Jun 18, 2023

mojavelinux commented Jun 18, 2023 •

edited

Loading

joostschriek commented Jun 19, 2023 •

edited

Loading

ggrossetie commented Jun 20, 2023 •

edited

Loading

ggrossetie commented Jun 20, 2023 •

edited

Loading

joostschriek commented Jun 20, 2023 •

edited

Loading

mojavelinux commented Jun 21, 2023 •

edited

Loading

ggrossetie commented Jun 21, 2023

joostschriek commented Jul 4, 2023

mojavelinux commented Jul 5, 2023

mojavelinux commented Jul 5, 2023

ggrossetie commented Jul 8, 2023

mojavelinux commented Jul 8, 2023

asciidoctor-kroki breaks when using remote content repositories #416

asciidoctor-kroki breaks when using remote content repositories #416

Comments

joostschriek commented Jun 16, 2023 • edited Loading

ggrossetie commented Jun 17, 2023

ggrossetie commented Jun 18, 2023

mojavelinux commented Jun 18, 2023

mojavelinux commented Jun 18, 2023

ggrossetie commented Jun 18, 2023

mojavelinux commented Jun 18, 2023 • edited Loading

joostschriek commented Jun 19, 2023 • edited Loading

ggrossetie commented Jun 20, 2023 • edited Loading

ggrossetie commented Jun 20, 2023 • edited Loading

Current output

Expected output

joostschriek commented Jun 20, 2023 • edited Loading

mojavelinux commented Jun 21, 2023 • edited Loading

ggrossetie commented Jun 21, 2023

joostschriek commented Jul 4, 2023

mojavelinux commented Jul 5, 2023

mojavelinux commented Jul 5, 2023

ggrossetie commented Jul 8, 2023

mojavelinux commented Jul 8, 2023

joostschriek commented Jun 16, 2023 •

edited

Loading

mojavelinux commented Jun 18, 2023 •

edited

Loading

joostschriek commented Jun 19, 2023 •

edited

Loading

ggrossetie commented Jun 20, 2023 •

edited

Loading

ggrossetie commented Jun 20, 2023 •

edited

Loading

joostschriek commented Jun 20, 2023 •

edited

Loading

mojavelinux commented Jun 21, 2023 •

edited

Loading