Azure / azure-functions-durable-js

JavaScript library for using the Durable Functions bindings
https://www.npmjs.com/package/durable-functions
MIT License
128 stars 46 forks source link

Module not found: Error: Package path ./v5 is not exported from package uuid #481

Closed steveacalabro closed 1 year ago

steveacalabro commented 1 year ago

Describe the bug This happens during a fresh install of the JS package durable-functions@2.1.1. When you attempt to build it with typescript you will get this as an error. My assumption is the wrong version of UUID is installed in the package itself. It is most likely related to this line of code https://github.com/Azure/azure-functions-durable-js/blob/2857fc579ba61b91673e5a4b259bf9ce2386bd1c/src/guidmanager.ts#L3.

Investigative information

To Reproduce Steps to reproduce the behavior:

  1. Go to a fresh azure function apps project using typescript
  2. Install the durable functions npm package using yarn
  3. Implement the function using the base example from the documentation website https://learn.microsoft.com/en-us/azure/azure-functions/durable/quickstart-js-vscode
  4. Build app
  5. 
    Module not found: Error: Package path ./v5 is not exported from package /home/runner/work/{project-name}/{project-name}/node_modules/uuid (see exports field in /home/runner/work/{project-name}/{project-name}/node_modules/uuid/package.json)
    @ ./node_modules/durable-functions/lib/src/classes.js 93:20-44
    @ ./node_modules/durable-functions/lib/src/index.js 4:18-38
    @ ./OrchestratorFunction/index.ts 27:24-52```

While not required, providing your orchestrator's source code in anonymized form is often very helpful when investigating unexpected orchestrator behavior.

// https://learn.microsoft.com/en-us/azure/azure-functions/durable/quickstart-js-vscode
import * as df from "durable-functions"

const orchestrator = df.orchestrator(function* (context) {
    const outputs = [];

    outputs.push(yield context.df.callActivity("ActivityName"));

    return outputs;
});

export default orchestrator;

Expected behavior A clear and concise description of what you expected to happen.

I expect to be able to build using typescript. I also would think that the package version should be as close to up-to-date as possible to eliminate any potential security threats

Actual behavior A clear and concise description of what actually happened.

The build errors locally and in CI

Screenshots If applicable, add screenshots to help explain your problem.

NA

Known workarounds Provide a description of any known workarounds you used.

davidmrdavid commented 1 year ago

@hossam-nasr: could you please prioritize taking a look at this?

hossam-nasr commented 1 year ago

@steveacalabro I wasn't able to repro your issue. Do you mind sharing your package.json file? Also, which version of uuid ended up being resolved in your yarn.lock/package-lock.json file? In my tests, I got version 3.3.3, and that compiled fine. In the node_modules folder, you could see that there is indeed a v5.js file in the root of the package.

steveacalabro commented 1 year ago

@hossam-nasr This may help. I was able to thin down my code to just these functions here in this stackblitz.

You should be able to reproduce using

yarn build

In the yarn.lock, the durable functions resolves to this

durable-functions@2.1.1:
  version "2.1.1"
  resolved "https://registry.yarnpkg.com/durable-functions/-/durable-functions-2.1.1.tgz#9cbb982ef88ab766267990fab95e33fba974e015"
  integrity sha512-/fCgtkLb2C5ugKCdIxxkJ/k2SFOYw8bnMte9EDb3a+p06HBkp2x+RbA9OzexR/rIvgj45T5m67rjsberial5pw==
  dependencies:
    "@azure/functions" "^1.2.3"
    axios "^0.21.1"
    debug "~2.6.9"
    lodash "^4.17.15"
    moment "^2.29.2"
    uuid "~3.3.2"
    validator "~13.7.0"
hossam-nasr commented 1 year ago

@steveacalabro Thanks for providing this info! I was able to get this problem to repro with the given package.json file. I noticed that you had this line:

"resolutions": {
    "uuid": "9.0.0"
  }

In your package.json file, which was forcing uuid to resolve to version 9.0.0, causing the error. Removing this part from the package.json file fixed the issue for me. Is there a reason that you added this?

I do agree though with your broader point about upgrading the uuid version we use. This is also a message that you get when trying to install the durable-functions SDK:

warning durable-functions > uuid@3.3.3: Please upgrade  to version 7 or higher.  
Older versions may use Math.random() in certain circumstances, which is known to be problematic.  
See https://v8.dev/blog/math-random for details.
steveacalabro commented 1 year ago

@hossam-nasr I had that there trying to do a workaround "to use yarn's resolutions feature"

I removed it in the stack blitz and I'm still getting the same error on the build

*Edit: Actually that is not true. I apologize, the first run of that seemed to be a cache issue with NPM. Once I re-installed it in stack blitz it did work. When I remove that in my local code I am getting the same error. I'm going to try to see what missing between what's in the stack blitz and what's in the code. I assume that it is something with how I am using webpack to transpile. If I'm able to get it to reproduce I'll post here again

hossam-nasr commented 1 year ago

@steveacalabro Sounds good, thank you. Please update here if you're able to reproduce.

steveacalabro commented 1 year ago

@hossam-nasr Appreciate the help here! I apologize this actually ended up being with the way I was doing my webpack build so I am going to close this issue :)

If you're interested, in this portion of my webpack script

resolve: {
  extensions: ['.tsx', '.ts', '.js'],
  plugins: [new TsconfigPathsPlugin()],
  modules: [
    path.resolve(__dirname, '..', 'node_modules'),
    path.resolve(__dirname, 'node_modules'),
  ],
}

I was accidentally resolving the node_modules from the parent directory before the current one. This was overriding the modules with bad versions which ultimately caused the issue.

steveacalabro commented 1 year ago

Though, as we said. The lower version of UUID as a dependency should likely be noted for update just in case

hossam-nasr commented 1 year ago

@steveacalabro Glad this got resolved! And yes, agreed. I've filed an issue for that here: #483