Closed joostschriek closed 1 year ago
Could you please tell me what is the value of {partial-prefix}
?
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?
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.
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.
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()
}
}
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.
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.
:warning: 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.
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
:
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.
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 })
[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": {
}
}
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.`)
And the cause is that
target.src.abspath
isundefined
: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?
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)).
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
.
@ggrossetie i made an attempt at spot fixing this issue, i'm sure you'll let me know if everything's ok :smiley:
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.
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.
@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
.
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 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;
During our builds we install all the required npm extensions;
we call the extensions like so
During our debugging we've figured out that local works but remote doesn't. We've tried the following remediations;
A remote playbook
A local playbook