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

Axios upgrade breaks non-Node runtimes #127

Closed
huw opened this issue Nov 14, 2023 · 14 comments · Fixed by #132
Closed

Axios upgrade breaks non-Node runtimes #127

huw opened this issue Nov 14, 2023 · 14 comments · Fixed by #132
Labels
bug Something isn't working

Comments

@huw
Copy link

huw commented Nov 14, 2023

Bug description

#122 breaks non-Node runtimes by introducing dependencies on Node internals which weren’t present in the previous version of Axios. This is because Axios v1 doesn’t provide the correct non-Node exports, which leads them to try and import a bunch of Node libraries, which they can’t.

A few potential fixes off the top of my head:

  1. Replace Axios with fetch (now that fetch is in all LTS versions, this might not be unreasonable; it would also mean non-Node runtimes don’t have to add fetch as a parameter when instantiating PostHog)
  2. Agitate Axios to fix Default exports for browser points to module that imports node modules axios/axios#5495
  3. Patch it
  4. Split the package into Node and non-Node in package.json#exports

(I posted this as a comment on that PR but I’ve upgraded it to an issue so it doesn’t get lost)

@huw huw added the bug Something isn't working label Nov 14, 2023
@marcfrankel
Copy link

@huw Do you have a patch for this? I'm trying to make one but struggling hard...

@huw
Copy link
Author

huw commented Dec 1, 2023

@marcfrankel Been waiting on guidance from the maintainers, in the meantime v2.1.2 v3.1.2 is compatible.

@marcfrankel
Copy link

@huw Version 2.1.2? That version doesn't seem to exist. Do you mean 3.1.2?

@huw
Copy link
Author

huw commented Dec 1, 2023

Yep, was working from memory

@marcfrankel
Copy link

Hmm because i still get the error on that version:

PostHog Debug error  [PostHogFetchNetworkError: Network error while fetching PostHog] {
  error: [TypeError: adapter is not a function],
  name: 'PostHogFetchNetworkError'
}
PostHog Debug flush [
  {
  distinct_id: '6c8899c4-b681-4d96-987f-6b0182530485',
  event: 'updateBillingAlert',
  properties: {
  enabled: true,
  hardStop: true,
  threshold: 732,
  $groups: { company: 'e5903eb5-006a-4d3f-a34d-71b9d8929f2e' },
  $lib: 'posthog-node',
  $lib_version: '3.1.2',
  $geoip_disable: true
},
  type: 'capture',
  library: 'posthog-node',
  library_version: '3.1.2',
  timestamp: '2023-12-01T23:46:20.325Z'
}

@marcfrankel
Copy link

I think in general my error is caused by the fetch failing but instead of rejecting in a serverless/edge environment it waits the full timeout and slows everything down. So far my best idea is just to recreate the base functionality I need directly using the API endpoints, but the fact no one has responded to you @huw in like two weeks is a super bad look for posthog.

@benjackwhite
Copy link
Collaborator

Ah the world of Node.js environments... Apologies for the delay in responding here. We're a small team doing our best to cover a wide range of issues.

I'd like to make sure we capture this for good so we don't get caught out in the future. I'll try and get some sort of fix out asap hopefully with some sort of tests that could catch this.

@huw
Copy link
Author

huw commented Dec 4, 2023

No complaints from me—maintaining an SDK for every language & runtime combo is tough work and non-Node is still pretty niche. Thanks for getting to it!

@benjackwhite
Copy link
Collaborator

@huw I don't suppose you have a simple way of reproducing this, short of deploying a full cloudflare worker...?

@huw
Copy link
Author

huw commented Dec 4, 2023

Actually yes! You could:

  1. Run Miniflare v2 in a Jest/Vitest environment
  2. Run Miniflare v3 using the unstable_dev() API (harder to configure, but much more accurate & includes updated features)
  3. Patch @types/node to an empty package and see if there are any compilation errors (sadly, dependencies are allowed to transitively pollute the namespace with @types/node so the only way to guarantee Node types will resolve to nothing is to patch it away—see Transitive loading of @types/node breaks Request/Response (etc.) types cloudflare/workerd#1298 and [node] Add fetch types DefinitelyTyped/DefinitelyTyped#66824 for more.)
  4. Compile the code and check for /// <reference type=“node” /> anywhere in the code (this is a less good check because sometimes things will transitively depend on @types/node but still compile and run in a Worker environment)

(I was just talking about testing strategies here but re-reading your comment, if you want to just test a Worker in development you can use Wrangler for that)

(I personally discovered this because I do (3) anyway for type correctness and my code refuses to compile)

@benjackwhite
Copy link
Collaborator

Just merged the change that should be a workaround for this issue (albeit not the end fix). Feel free to re-open if it is still an issue on the latest version

@maxprilutskiy
Copy link

@benjackwhite FYI, i've got this problem yesterday: #151

@sonnyvesali
Copy link

I also had this problem when trying to use posthog in supabase edge functions & trying to deploy it, downgrading to version 3.1.2 worked for me.

@pellicceama
Copy link

This happens to me on posthog-node ^4.0.0. Downgrading to version 3.1.2 also worked for me.

/app/api/node_modules/posthog-node/node_modules/axios/index.js:1
    ({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){import axios from './lib/axios.js';
                                                                                             ^^^^^^

    SyntaxError: Cannot use import statement outside a module

      at Runtime.createScriptFromCode (node_modules/jest-runtime/build/index.js:1350:14)
      at Object.<anonymous> (node_modules/posthog-node/src/fetch.ts:21:17)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants