marp-team / marp-cli

A CLI interface for Marp and Marpit based converters
MIT License
1.9k stars 108 forks source link

`Failed converting Markdown. (Protocol error (Target.setDiscoverTargets): Target closed.)` in AWS ECS #543

Closed yutak23 closed 1 year ago

yutak23 commented 1 year ago

I did not reproduce this when I built the Docker image and docker run in my local environment, but when I deployed the container with the same Docker image to AWS ECS, the following error occurred.

Failed converting Markdown. (Protocol error (Target.setDiscoverTargets): Target closed.)

I debugged it and found that isDocker(https://github.com/marp-team/marp-cli/blob/main/src/utils/docker.ts#L3) was true in the local environment, but false in the ECS. I think it is because the is-docker package is not correctly determining that it is a Docker container.

environment

Dockerfile

FROM node:16.19.1-alpine3.16
 
RUN mkdir /app
WORKDIR /app
 
COPY package.json yarn.lock ./
RUN yarn install --frozen-lockfile
 
COPY . .
 
RUN yarn build && yarn express:build
 
RUN apk update && apk upgrade && \
        apk add --no-cache chromium
 
ENTRYPOINT ["/bin/sh","./entrypoint.sh"]
EXPOSE 3000

I use Marp CLI through Node.js. (https://github.com/marp-team/marp-cli#api-experimental)

I already referenced

Tentative error resolution I have found.

From the following code, isDocker is true even if the environment variable MARP_USER is present. https://github.com/marp-team/marp-cli/blob/main/src/utils/docker.ts#L4

In other words, the error could be resolved by setting the environment variable MARP_USER in a hackish way, although it is not an official image.

...
RUN apk update && apk upgrade && \
        ...

ENV MARP_USER=marp_user # <- add setting environment variable 
 
ENTRYPOINT ["/bin/sh","./entrypoint.sh"]
...
yhatt commented 1 year ago

I'm not very familiar with ECS, but do you think it will work if swapped is-docker to is-inside-container?

yutak23 commented 1 year ago

I'm not very familiar with ECS, but do you think it will work if swapped is-docker to is-inside-container?

It might solve the problem, but unfortunately is-inside-container is a pure ESM package, so I don't think it is available in marp-cli which is exposed as CommonJS...

If you would like, I can create a PR to publish the ES Module along with CommonJS as well.

yhatt commented 1 year ago

is-inside-container is a pure ESM package, so I don't think it is available in marp-cli which is exposed as CommonJS...

Marp CLI has already used a lot of pure ESM packages, including is-docker v3. There should be no barriers to replacement. :)

yutak23 commented 1 year ago

Marp CLI has already used a lot of pure ESM packages, including is-docker v3. There should be no barriers to replacement. :)

I think it is because is-docker is bundled and not required when building by rollup. In fact, in the post-build code, the is-docker code is bundled.

image

I think the reason it is bundled is because is-docker only depends on node's standard modules.

However, is-inside-container is not bundled because it depends on is-docker. In fact, if you make the following modifications and run yarn build:standalone, you will see that there is require("is-inside-container").

// src/utils/docker.ts
// import _isDocker from 'is-docker';
import isInsideContainer from 'is-inside-container';

export const isDocker = () => isOfficialImage() || isInsideContainer();
export const isOfficialImage = () => !!process.env.MARP_USER;

image

Thus, running node marp-cli.js will result in the following error.

$ node marp-cli.js 
...

Error [ERR_REQUIRE_ESM]: require() of ES Module /home/study/workspace/marp-cli/node_modules/is-inside-container/index.js from /home/study/workspace/marp-cli/lib/marp-cli-d775c9f3.js not supported.
Instead change the require of index.js in /home/study/workspace/marp-cli/lib/marp-cli-d775c9f3.js to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/home/study/workspace/marp-cli/lib/marp-cli-d775c9f3.js:1:383)
    at Object.<anonymous> (/home/study/workspace/marp-cli/lib/marp-cli.js:1:75)
    at Object.<anonymous> (/home/study/workspace/marp-cli/marp-cli.js:5:1) {
  code: 'ERR_REQUIRE_ESM'
}

Therefore, if we want to use is-inside-container, we will need to make your code ES Module after the build.

yhatt commented 1 year ago

Therefore, if we want to use is-inside-container, we will need to make your code ES Module after the build.

We don't have to need to ship CLI as ESM. Check out https://github.com/marp-team/marp-cli/pull/544. rollup will bundle is-inside-container correctlly :)

yhatt commented 1 year ago

Released new version v3.2.1, that is using is-inside-container instead of is-docker to detect virtualized containers.

Could you try it? If ECS used Podman to host the container, it may mitigate the issue brought by Chromium and its sandbox.

yutak23 commented 1 year ago

Perhaps there was an error in perception on my part.

Thank you very much. I will check it out.

yutak23 commented 1 year ago

We don't have to need to ship CLI as ESM. Check out https://github.com/marp-team/marp-cli/pull/544. rollup will bundle is-inside-container correctlly :)

I guess I didn't understand about the rollup behavior, if I set is-inside-container in devDependencies, it is indeed bundled, so no problem. My sincere apologies.

Could you try it? If ECS used Podman to host the container, it may mitigate the issue brought by Chromium and its sandbox.

I updated the version and Deployed to ECS to check, but got an error. However, the error message changed to the following. Failed converting Markdown.(Protocol error (Target.setAutoAttach): Target closed)

yhatt commented 1 year ago

Try updating your Dockerfile to run Chrome as non root user, as does in official images such as Marp CLI and Puppeteer. https://stackoverflow.com/questions/75495565/puppeteer-docker-target-setautoattach-target-is-closed

It's also helpful for debug to set DEBUG env as puppeteer:*.

yutak23 commented 1 year ago

Thank you. I will try, it may take some time due to circumstances.

yutak23 commented 1 year ago

I thought the error had changed, but the debug log shows that the error has not changed. Here are the details of the error logs.

puppeteer:error [
TargetCloseError: Protocol error (Target.setDiscoverTargets): Target closed
at CallbackRegistry.clear(/home/marp/app/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Connection.js:138:36)
at Connection.#onClose(/home/marp/app/node_modules/puppeteer-core/lib/cjs/puppeteer/common/Connection.js:282:25)
at Socket.<anonymous> (/home/marp/app/node_modules/puppeteer-core/lib/cjs/puppeteer/node/PipeTransport.js:24:34)
at Socket.emit (node:events:525:35)
at Pipe.<anonymous> (node:net:301:12)
]
puppeteer:protocol:SEND► [ '{"method":"Browser.close","id":3}' ]
{"time":"2023-08-..........","error":{"message":"Failed converting Markdown. (Protocol error (Target.setAutoAttach): Target closed)","stack":"CLIError: Failed converting Markdown. (Protocol error (Target.setAutoAttach): Target closed)\n ...","status":null}}
puppeteer:browsers:launcher Browser process 56 onExit
yhatt commented 1 year ago

is-inside-container is intended to identify common container services used to run the built image, but it might not work within some AWS infrastructures, that are opaque such as App Runner or Fargate.

AWS ECS, that is mentioned at the subject, basically just hosts the built container image, so it is unlikely that ECS is the root cause. What AWS service are you using to run the image?

If the workaround the MARP_USER environment variable was helpful, is helpful providing more explicit setting, such as the PUPPETEER_NO_SANDBOX environment variable?

yutak23 commented 1 year ago

AWS ECS, that is mentioned at the subject, basically just hosts the built container image, so it is unlikely that ECS is the root cause. What AWS service are you using to run the image?

I use Fargate in that sense.

If the workaround the MARP_USER environment variable was helpful, is helpful providing more explicit setting, such as the PUPPETEER_NO_SANDBOX environment variable?

Yes, I believe so.

yhatt commented 1 year ago

v3.3.0 provides CHROME_NO_SANDBOX env to opt out Chrome sandbox explicitly.