dart-lang / setup-dart

A GitHub Action to install and setup a Dart SDK.
BSD 3-Clause "New" or "Revised" License
192 stars 38 forks source link

issues with the code for this action and the new javascript interop #107

Closed devoncarew closed 11 months ago

devoncarew commented 1 year ago

When trying the new (unpublished) latest patch release functionality we've run across a few issues.

longer failure: ``` Installing the linux-x64 Dart SDK version 3 from the stable (release) channel. https://storage.googleapis.com/dart-archive/channels/stable/release/3/sdk/dartsdk-linux-x64-release.zip ... Error: Error: Unexpected HTTP response: 404 /home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d96812[5](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:6)f[6](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:7)8406/dist/main.cjs:3975 throw error; ^ TypeError: Cannot set property exitCode of [object Module] which has only a getter at Module.setFailed (file:///home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/index.mjs:292:22) at Object._fail (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:6140:23) at /home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:5993:1[7](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:8) at _wrapJsFunctionForAsync_closure.$protected (/home/runner/work/_actions/dart-lang/setup-dart/[8](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:9)ef13bcca6c[9](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:10)bb221939[10](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:11)88bc9d968125f68406/dist/main.cjs:3385:15) at _wrapJsFunctionForAsync_closure.call$2 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7552:12) at _awaitOnObject_closure0.call$2 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7546:25) at _RootZone.runBinary$3$3 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7967:18) at _FutureListener.handleError$1 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7606:21) at _Future__propagateToListeners_handleError.call$0 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7904:49) at Object._Future__propagateToListeners (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:3498:77) { dartException: UnknownJsTypeError { '__js_helper$_message': 'Cannot set property exitCode of [object Module] which has only a getter', '$thrownJsError': TypeError: Cannot set property exitCode of [object Module] which has only a getter at Module.setFailed (file:///home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968[12](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:13)5f68406/dist/index.mjs:292:22) at Object._fail (/home/runner/work/_actions/dart-lang/setup-dart/8ef[13](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:14)bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:6[14](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:15)0:23) at /home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:5993:17 at _wrapJsFunctionForAsync_closure.$protected (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:3385:[15](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:16)) at _wrapJsFunctionForAsync_closure.call$2 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7552:12) at _awaitOnObject_closure0.call$2 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7546:25) at _RootZone.runBinary$3$3 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7967:18) at _FutureListener.handleError$1 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7606:21) at _Future__propagateToListeners_handleError.call$0 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7904:49) at Object._Future__propagateToListeners (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:3498:77) { '$cachedTrace': _StackTrace { _exception: [Circular *1], _trace: 'TypeError: Cannot set property exitCode of [object Module] which has only a getter\n' + ' at Module.setFailed (file:///home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/index.mjs:292:22)\n' + ' at Object._fail (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:6140:23)\n' + ' at /home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:5993:[17](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:18)\n' + ' at _wrapJsFunctionForAsync_closure.$protected (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:3385:15)\n' + ' at _wrapJsFunctionForAsync_closure.call$2 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7552:12)\n' + ' at _awaitOnObject_closure0.call$2 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7546:25)\n' + ' at _RootZone.runBinary$3$3 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:7967:[18](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:19))\n' + ' at _FutureListener.handleError$1 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb22[19](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:20)391088bc9d968125f68406/dist/main.cjs:7606:[21](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:22))\n' + ' at _Future__propagateToListeners_handleError.call$0 (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb[22](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:23)19391088bc9d9681[25](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:26)f68406/dist/main.cjs:7904:49)\n' + ' at Object._Future__propagateToListeners (/home/runner/work/_actions/dart-lang/setup-dart/8ef13bcca6c9bb2219391088bc9d968125f68406/dist/main.cjs:[34](https://github.com/devoncarew/pkgs/actions/runs/6347104032/job/17241619443?pr=4#step:3:35)98:77)' } } } } ```
devoncarew commented 1 year ago

Likely related, we can call the process.exitCode setter, but it does not properly change the exit code of the process (i.e., even after callingprocess.exitCode = 1; the process will still exit w/ a code of 0.

devoncarew commented 1 year ago

cc @srujzs - we're having some trouble with some of the interop code; one method throws when we try and call it and one method doesn't seem to properly modify a javascript value. Perhaps due to javascript modules in some way? I've mostly worked around this in https://github.com/dart-lang/setup-dart/pull/108 but it would be nice to be able to call the normal github action apis. Happy to provide more details as necessary.

srujzs commented 1 year ago

exitCode seems to be only imported as a getter when I print out the process module. I think the reason why it throws in setFailed versus a silent no-op when setting it directly might be due to a difference in strict-mode, but not sure. I wonder if setting exitCode fails because there's a difference in how process is imported e.g. import process from 'process' vs import * as process from 'process', where the latter is sealed.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#default_import https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import#namespace_import

I wanted to try out the different import, but couldn't figure how to regenerate index.mjs.

devoncarew commented 1 year ago

Thanks for the response - sounds like there may be some things I can investigate wrt the imports.

I wanted to try out the different import, but couldn't figure how to regenerate index.mjs.

You can re-compile the javascript and re-bundle it via running npm run all (some of the details of how the action is built are in the https://github.com/dart-lang/setup-dart/blob/main/DEVELOPING.md doc).

srujzs commented 1 year ago

Ah I missed that - thanks!

Changing the imports does get me past that error now.

devoncarew commented 1 year ago

Thanks! W/ the import change I'm now able to assign a value to process exitCode (https://github.com/dart-lang/setup-dart/pull/108/files#diff-9fb9cb088ba78f6a223d4643987e84f1ca1fec8741ad34c2912176aad9a2d0c6R12).

srujzs commented 1 year ago

Nice! I think you should be able to call setFailed and avoid having to deal with the process API directly, do you notice an issue still with that?

devoncarew commented 1 year ago

I did not re-try that API. I believe that's from the @actions/core module? I wasn't really sure if I should convert all the import types over to default imports or keep them as namespace imports; I just tweaked the one (for process) that I was having trouble with.

srujzs commented 1 year ago

I believe that's from the @actions/core module

Yeah, I think that's right. It looks like all it does is set the exitCode and error with the given message, so it should work without any other import changes.