import() are not supported for service workers

[Mikescops]

Description

• Node.js version: 18
• Gitbeaker version: 39.28.0
• Gitbeaker release (cli, rest, core, requester-utils): rest
• OS & version: MacOS latest

Since the move from @gitbeaker/browser to @gitbeaker/rest it seems like import() has been added to the code. The problem is that this is not supported inside browser extensions and service workers at large.

See https://github.com/w3c/webextensions/issues/212 for instance.

Here is one of the offensive line:

node_modules/@gitbeaker/rest/dist/index.mjs:9:38
     8 |     if (typeof window !== "object") {
  >  9 |       const { Agent } = await import('https');
  >    |                                      ^^^^^^^
    10 |       options.agent = new Agent({
    11 |         rejectUnauthorized: false

Steps to reproduce

Use gitbeaker/rest inside a service work.

Expected behaviour

Ideally should not use dynamic imports.

Actual behaviour

Uses dynamic imports.

Checklist

• [x] I have checked that this is not a duplicate issue.
• [x] I have read the documentation.

[jdalrymple]

Hmm, browser extensions should be included as a browser environment. That line should never be encountered when running the library. Is it being hit in your use case? and if so, how can i filter out browser extensions environments since they apparently are not setting the global window object.

Dynamic import themselves are not an issue, as they are supported within browser, deno and node environments. Its solely in the context of service workers that it causes an issue.

[kingmesal]

I'm running into this same issue

Is the import itself the problem?...

The error I get: ✘ [ERROR] Could not resolve "https"

[kingmesal]

So, I did the following:

// src/index.ts
async function defaultOptionsHandler(resourceOptions, requestOptions) {
  const options = { ...requestOptions };
  // if (resourceOptions.url.includes("https") && resourceOptions.rejectUnauthorized != null && resourceOptions.rejectUnauthorized === false) {
  //   if (typeof window === "undefined") {
  //     const { Agent } = await import('https');
  //     options.agent = new Agent({
  //       rejectUnauthorized: false
  //     });
  //   }
  // }
  return options;
}

And all the API calls I make to gitlab.com worked great!

Given that this is a REST api, the fetch commands have to already support https can this be removed?

[jdalrymple]

Hmm you may be right, let me dive into the latest support for rejectUnauthorized and follow up

[jdalrymple]

Looks like this support was implemented incorrectly by me! Ill make a pr that should sort this -> https://github.com/nodejs/undici/issues/1489#issuecomment-1543856261

[jdalrymple]

The latest PR does fix the actual functionality, but doesnt fix your issue I dont think. Since the problem lies with the dynamic import and not the actual library. Feel free to give it a look though. I could be wrong :shrug:

[jdalrymple]

Now that i think about it, one could really just use the env var:

process.env.NODE_TLS_REJECT_UNAUTHORIZED = 0;

*EDIT: https://github.com/jdalrymple/gitbeaker/blob/c0ad1a7077833e0aa2222bcae7e558c85b5c4283/docs/FAQ.md?plain=1#L28

Not sure if this is still the case, would have to test it

[kingmesal]

I think using process would bring another issue because that is a node feature.

I really do not think the dynamic import is the problem, I think it is the reference to a node library that is not available in cloudflare workers.

On Sun, Feb 4, 2024, 5:25 PM Justin Dalrymple @.***> wrote:

Now that i think about it, one could really just use the env var:

process.env.NODE_TLS_REJECT_UNAUTHORIZED = 0;

...

— Reply to this email directly, view it on GitHub https://github.com/jdalrymple/gitbeaker/issues/3505#issuecomment-1925946238, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB2Q7ULS3MEY7KPQN444HJTYSADGJAVCNFSM6AAAAABBS3RG4OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRVHE2DMMRTHA . You are receiving this because you commented.Message ID: @.***>

[jdalrymple]

I think you misunderstood. I'm saying the functionality you commented out should be able to be supported by users if they just set an environment variable.

To do this in node is the process.env.. command you saw above, but you can do this any which way you want in the environment you're in.

If you don't think the dynamic import is the issue, check out that pr branch and see if it runs fine for you.

[kingmesal]

Oh, yeah, that sounds like a great plan with the env variable.

I just tested swapping in the undici library and wow, that throws a littany of issues because there are deeper chains of dependencies that workers doesn't like.

Maybe that dynamic import is just ultimately problematic....

[jdalrymple]

Darn that's what I figured. I'll try and test an example for the env var. If it works fine I'll remove the code you commented out

[kingmesal]

After some playing around and thinking about what is happening I think I can now explain the issue.

Cloudflare workers get "pre" compiled to be deployed remotely or run locally. When it goes through that compilation it needs to prebake every dependency. So any / all imports in the code are required to be loaded into that compiled worker. Thus, it isn't that it is a dynamic import, it is that the compilation step sees it and it doesn't need to determine whther or not the code can ever be run, it is loading everything in case it does get run later. So, the fact that cloudflare workers run on V8 and not node, when there are VERY deep dependency trees (like undici) it becomes even harder to build a code base that can be deployed.

Hopefully that is helpful to, well, someone...

[Mikescops]

Hey folks, Thanks for the investigation! I confirm what @kingmesal just said, I use Parcel to build my webextension and indeed it doesn't read the conditions so all imports are considered at build time and that's where it breaks.

[kingmesal]

Any luck on testing that @jdalrymple ?

[jdalrymple]

Not yet sorry! Still trying to think of the best way to do that.

[kingmesal]

Is there any help I could provide to move this over the line?

[jdalrymple]

If you can think of a good way to validate if setting this env: NODE_TLS_REJECT_UNAUTHORIZED = '0', allows for unauthorized gitlab api connections through gitbeaker that would be solid haha Thats all i need to test before i can remove that section of code code completely.

[kingmesal]

Are there any specific end points that are able to be used to test it against?

Is there and example of how it is suppose to act in the use case documented anywhere?

[Mikescops]

I'm wondering if it's visible anywhere in the HTTP call that is fired, but I highly doubt about it.

The best solution would be to have a live end to end test, but that's way trickier to setup.

I'll have a look on my end too.

[kingmesal]

Would it be feasible to startup a nodejs server with an expired, invalid, etc.. self-signed certificate that hosts a single test endpoint that matches a basic endpoint from the gitlab api? Could it be mocked out?

[jdalrymple]

I'm wondering if it's visible anywhere in the HTTP call that is fired, but I highly doubt about it.

The best solution would be to have a live end to end test, but that's way trickier to setup.

I'll have a look on my end too.

So i have e2e tests setup, testing against an instance with unsecured endpoints (http). I think i would need to spin up an instance and use a self signed cert for the server and then validate against that. This comment: https://github.com/jdalrymple/gitbeaker/issues/357#issuecomment-567523037, suggests the env would be sufficient, but that was 5 years ago so probably should be verified.

Here are some related issues that may shed more light on how to validate this #357

[jdalrymple]

Would it be feasible to startup a nodejs server with an expired, invalid, etc.. self-signed certificate that hosts a single test endpoint that matches a basic endpoint from the gitlab api? Could it be mocked out?

Yea this should be okay!

[jdalrymple]

Are there any specific end points that are able to be used to test it against? Is there and example of how it is suppose to act in the use case documented anywhere?

Not really, just want to ensure the bypass works correctly. It should, but if i am going to document it, i want to sure.

[kingmesal]

through a little bit of "head scratching" and testing, I have found a solution for cloudflare workers.

      const { Agent } = await import('node:https');

Adding that node: prefix to the library resolves this issue ...

When coupled with adding this when running the wrangler command: --compatibility-flags='nodejs_compat'

And adding this to wrangler.toml: compatibility_flags = [ "nodejs_compat" ]

My compilation problems have disappeared.

That change would unblock anyone using gitbeaker within a workers context.

[Mikescops]

That change would unblock anyone using gitbeaker within a workers context.

cloudflare workers*

[jdalrymple]

I found a good way to test this, ill try it this week: https://github.com/danieleagle/gitlab-https-docker/blob/master/README.md

[kingmesal]

@jdalrymple any luck?

[kingmesal]

This may sound like a selfish ask, and perhaps it is, but it would prevent me from having to create a temporary fork to get around this. Any chance you could get this suggested change in if the testing of NODE_TLS_REJECT_UNAUTHORIZED will be much longer?

      const { Agent } = await import('node:https');

[jdalrymple]

Sorry for the delay, just been a busy couple weeks. unfortunately that is the wrong import all together. So its either switch it with the right one from undici or remove it all together. Im aiming for the latter which would properly solve this issue, but just havent had the time to test it. I was hoping one of the commenters who inquired might have had the chance to test it on their own system which would of saved me time spinning up a new instance and self signing it, but I havent received a response as of yet

[kingmesal]

No worries about delays, I get it.

On the first point, I'm not sure I follow that it is the wrong library, the https library provided by is documented to be able to be pulled in with import node:https https://nodejs.org/api/https.html. Or are you saying there is another https library that is not provided by node that is being imported?

I've been reading more about the flag NODE_TLS_REJECT_UNAUTHORIZED and I feel like trying to validate this functionality is literally validating that NODE JS has implemented its documented API. That documentation is pretty straight forward.

Do you feel is is really important to run a test to validate the nodejs functionality?

And if so, I found this site provided web servers with a variety of different ssl configurations available: https://badssl.com/

Is it reasonable to point gitbeaker at one of those just to try to auth? It should fail with a bad cert immediately, then when that flag is set it should fail because there is API to respond, right?