Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use in Workers because of process in userAgent.ts #658

Closed
jfsiii opened this issue May 5, 2022 · 14 comments · Fixed by #899
Closed

Cannot use in Workers because of process in userAgent.ts #658

jfsiii opened this issue May 5, 2022 · 14 comments · Fixed by #899
Labels

Comments

@jfsiii
Copy link

jfsiii commented May 5, 2022

Describe the bug

I tried running this in a Worker using both Miniflare & Wrangler and they crashed with ReferenceError: process is not defined

The stack traces point to

export const userAgent = `datadog-api-client-typescript/${version} (node ${process.versions.node}; os ${os.type()}; arch ${os.arch()})`

To Reproduce
Steps to reproduce the behavior:

  1. npm install @datadog/datadog-api-client
  2. import & use like:
    import { v1 } from '@datadog/datadog-api-client';
    const configuration = v1.createConfiguration();
    const apiInstance = new v1.LogsApi(configuration);
    
  3. build site (miniflare dev or wrangler dev
  4. observe ReferenceError: process is not defined and process exit

Expected behavior
library functions won't crash

I wasn't sure what the changes to the UA string should be

  • Add workers or some other info about the runtime?
  • only add that if there is a node version
  • something else

Environment and Versions (please complete the following information):
A clear and precise description of your setup:

  • version for this project in use: @datadog/datadog-api-client@1.0.0-beta.9
  • services, libraries, languages and tools list and versions wrangler@1.19.12
@jfsiii jfsiii added the kind/bug label May 5, 2022
@jfsiii jfsiii changed the title Cannot use in Workers because of process.node Cannot use in Workers because of process.versions.node in userAgent.ts May 5, 2022
@jfsiii jfsiii changed the title Cannot use in Workers because of process.versions.node in userAgent.ts Cannot use in Workers because of process in userAgent.ts May 5, 2022
@therve
Copy link
Contributor

therve commented May 5, 2022

Thanks for the report. I don't know much about wrangler, I tried a basic module and it failed with XMLHttpRequest is not defined. Is there something else I'm missing in the basic setup? Can you provide the small reproducing worker code?

@jfsiii
Copy link
Author

jfsiii commented May 9, 2022

@therve Thankfully it just got a lot easier to setup & run a worker.

I'm going to update description to show how to use these on a brand new project. The error changed (it now fails because of the node-specific os) but the issue / request is the same (don't crash on non-node runtimes if possible) Actually, after setting up the repro example, there's a few issues at work, so I'm just going to leave some notes here and leave it up to you want to deal with this ticket.

Working workers example

  1. npx wrangler init some-workers-example-repo -y
  2. cd some-workers-example-repo
  3. npx wrangler dev src/index.ts
    ⛅️ wrangler 2.0.1
    -------------------
    ⬣ Listening at http://localhost:8787
    12:05:21 PM GET / 200
    Script modified; context reset.
    12:05:21 PM GET /favicon.ico 200
    ╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
    │ [b] open a browser, [d] open Devtools, [l] turn on local mode, [c] clear console, [x] to exit                                                                  │
    ╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
    
  4. press b or otherwise visit http://localhost:8787 and observe "Hello world" response

Crashes with @datadog/datadog-api-client

  1. npm add @datadog/datadog-api-client
  2. Setup client instance, e.g.
    diff --git a/src/index.ts b/src/index.ts
    index 0ddd687..3ba0f21 100644
    --- a/src/index.ts
    +++ b/src/index.ts
    @@ -8,6 +8,11 @@
    * Learn more at https://developers.cloudflare.com/workers/
    */
    
    +import { v1 } from '@datadog/datadog-api-client';
    +
    +const configuration = v1.createConfiguration();
    +const apiInstance = new v1.LogsApi(configuration);
    +
    export default {
    async fetch(request: Request): Promise<Response> {
        return new Response("Hello World!");
  3. run npx wrangler dev src/index.ts again
  4. observe error message
✘ [ERROR] Could not resolve "os"

    node_modules/@datadog/datadog-api-client/dist/userAgent.js:23:30:
      23 │ var os = __importStar(require("os"));
         ╵                               ~~~~

  The package "os" wasn't found on the file system but is built into node. Are you trying to bundle
  for node? You can use "platform: 'node'" to do that, which will remove this error.

My attempts to resolve

I a few iterations of "run wrangler, patch the error, repeat" which was mainly prefacing all process access with typeof process !== 'undefined'. There's no checks in userAgent and a few other places (packages/*v1/configuration ?) do process !== undefined which fails with a ReferenceError.

After patching those I hit the XMLHttpRequest is not defined error you mentioned. I believe that's due to lquixada/cross-fetch#78. Here it is failing with the same error in a Chrome extension (also a worker context)

So it seems there are two areas at work

  1. Node specific vars like process or built ins like os in @datadog/datadog-api-client library code
  2. Cross-fetch is not usable in service workers lquixada/cross-fetch#78

I think you can work around (1) an isNode guard of some type, like:

const isNode = () =>
  typeof process !== "undefined" &&
  process.versions != null &&
  process.versions.node != null;

In the case of userAgent is seems reasonable to have things still work even if we're not in node. The behavior won't change. Perhaps other places aren't as obvious when we not in node.

For (2), I was going to suggest replacing cross-fetch with something like node-fetch but I see you just recently did the reverse :) #498

Regardless, at least now you should be able to easily create a worker environment and have confidence when you're testing ideas / changes.

@maurerbot
Copy link

Any progress on this?

@maurerbot
Copy link

maurerbot commented Jun 7, 2022

I got this working by doing the following making a copy of https://github.com/DataDog/datadog-api-client-typescript/blob/master/packages/datadog-api-client-v1/http/isomorphic-fetch.ts in my project and using it as my own httpApi.

import datadoghttp from './datadog-poly'
import { v1 } from '@datadog/datadog-api-client'

const configuration = v1.createConfiguration({
  httpApi: new datadoghttp()
}); 

Haven't fully tested it but I got a nice 403 from Datadog. What this does specifically is remove the cross-fetch import in favor of the default fetch api.

@github-actions
Copy link

github-actions bot commented Jul 8, 2022

Thanks for your contribution!

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of this project.

  2. Comment that the issue is still reproducible and include updated details requested in the issue template.

@github-actions github-actions bot added the stale label Jul 8, 2022
@segevfiner
Copy link

I can't even use this in the browser due to this. This is caused by some modules importing userAgent.ts which does this process check unconditionally at import time.

@github-actions github-actions bot removed the stale label Sep 14, 2022
@github-actions
Copy link

Thanks for your contribution!

This issue has been automatically marked as stale because it has not had activity in the last 30 days. Note that the issue will not be automatically closed, but this notification will remind us to investigate why there's been inactivity. Thank you for participating in the Datadog open source community.

If you would like this issue to remain open:

  1. Verify that you can still reproduce the issue in the latest version of this project.

  2. Comment that the issue is still reproducible and include updated details requested in the issue template.

@github-actions github-actions bot added the stale label Oct 14, 2022
@c-ameron
Copy link

This is still relevant :)

@skarimo
Copy link
Member

skarimo commented Nov 7, 2022

👋 hello, we just merged #899

Runtimes where XHR is not available, users will need to supply their own implementation of HttpLibrary. See example implementation with proxy here: https://github.com/DataDog/datadog-api-client-typescript#configure-proxy. This is due to cross-fetch assuming XHR is always present lquixada/cross-fetch#78.

Otherwise, you can add polyfils. Looks like node_compat = true in the wrangler settings should do the trick.

@jfsiii
Copy link
Author

jfsiii commented Nov 7, 2022

users will need to supply their own implementation of HttpLibrary ...
Otherwise, you can add polyfils

ok, thank you. I won't do either of those but I appreciate the update

@skarimo
Copy link
Member

skarimo commented Nov 9, 2022

Another follow up. We just merged #900. This should use native fetch method on non-node environments, omitting the requirement of custom HttpLibrary implementation in environments where XHR is not available. Thanks

@jfsiii
Copy link
Author

jfsiii commented Nov 9, 2022

This should use native fetch method on non-node environments

Oh, this sounds perfect. Will this ticket or #900 get updated when there's a release ready with the fix?

@skarimo
Copy link
Member

skarimo commented Nov 10, 2022

I will update this issue once the next release happens

@skarimo
Copy link
Member

skarimo commented Nov 16, 2022

Hi all, quick update we just released v1.6.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants