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

[BUG] trap hook terminated #6147

Closed
2 tasks done
diachedelic opened this issue Feb 8, 2023 · 5 comments
Closed
2 tasks done

[BUG] trap hook terminated #6147

diachedelic opened this issue Feb 8, 2023 · 5 comments
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 9.x work is associated with a specific npm 9 release

Comments

@diachedelic
Copy link

diachedelic commented Feb 8, 2023

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

Scripts run via npm run have their trap hooks terminated prematurely. This is undesirable because the script does not get a chance to clean up after itself. Worse, it might clean up only some of its mess.

Expected Behavior

Any Bash "trap" hooks to run to completion.

Steps To Reproduce

Create a file demo.sh containing

#!/bin/bash
trap "echo start; sleep 1; echo end" EXIT
cat

and make it executable with chmod +x demo.sh. Notice that it has a trap hook that runs on exit. The hook echoes "start", does some work, then echoes "end". Declare it as the script "demo" in a package.json file:

{
    "scripts":{
        "demo":"./demo.sh"
    }
}

Run the "demo" script, then interrupt it by typing ctrl-c. Notice that the trap hook does not run to completion:

$ npm run demo

> demo
> ./demo.sh

^Cstart

On the other hand, running the script directly runs the trap hook to completion:

$ ./demo.sh
^Cstart
end

Environment

  • npm: 9.4.0
  • Node.js: v19.6.0
  • OS Name: MacOS 13.1
  • System Model Name: MacBook Pro
  • npm config:
; "user" config from /Users/me/.npmrc

//registry.npmjs.org/:_authToken = (protected) 

; node bin location = /nix/store/fnhfi19p4wlj38wjrxipnb355jxf3zzn-nodejs-19.6.0/bin/node
; node version = v19.6.0
; npm local prefix = /private/tmp
; npm version = 9.4.0
; cwd = /private/tmp
; HOME = /Users/me
; Run `npm config ls -l` to show all defaults.
@diachedelic diachedelic added Bug thing that needs fixing Needs Triage needs review for next steps Release 9.x work is associated with a specific npm 9 release labels Feb 8, 2023
@fritzy fritzy removed the Needs Triage needs review for next steps label Mar 2, 2023
@fritzy
Copy link
Contributor

fritzy commented Mar 2, 2023

I'm not able to reproduce this in [email protected]. Please reopen if this is still an issue.

projects/scratchpad/test3 via ⬢ v19.0.1 
❯ npm -v          
9.6.0

projects/scratchpad/test3 via ⬢ v19.0.1 
❯ cat package.json
{
  "name": "test3",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "demo": "./test.sh"
  },
  "keywords": [],
  "author": "",
  "license": "ISC"
}

projects/scratchpad/test3 via ⬢ v19.0.1 
❯ npm run demo

> [email protected] demo
> ./test.sh

^Cstart
end

@fritzy fritzy closed this as completed Mar 2, 2023
@fritzy fritzy reopened this Mar 2, 2023
@fritzy fritzy added the Priority 2 secondary priority issue label Mar 2, 2023
@fritzy
Copy link
Contributor

fritzy commented Mar 2, 2023

Looks like this can still happen in some cases.

@diachedelic
Copy link
Author

Yes, I'm still seeing it with NPM v9.6.0.

$ node_modules/.bin/npm run demo

> demo
> ./demo.sh

^Cstart

$ node_modules/.bin/npm -v
9.6.0

@fritzy
Copy link
Contributor

fritzy commented Mar 2, 2023

npm/run-script#142

@diachedelic
Copy link
Author

Seems to be fixed in NPM v9.7.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 9.x work is associated with a specific npm 9 release
Projects
None yet
Development

No branches or pull requests

2 participants