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

AddNpm App ResourceStop not working #6221

Closed
1 task done
swissarmykirpan opened this issue Oct 10, 2024 · 9 comments · Fixed by #6293
Closed
1 task done

AddNpm App ResourceStop not working #6221

swissarmykirpan opened this issue Oct 10, 2024 · 9 comments · Fixed by #6293
Assignees
Milestone

Comments

@swissarmykirpan
Copy link

swissarmykirpan commented Oct 10, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

I am executing the SWA CLI via NPM

      builder.AddNpmApp("swa", "app", "swa")
          .WithReference(api)
          .WithHttpEndpoint(targetPort: 4280);

Everything works fine on first startup. However when I stop Visual Studio, the node process is still running. Also if I press the stop button on the dashboard, it reports:

2024-10-10T02:25:19 Executing command 'resource-stop'.
2024-10-10T02:25:19 Successfully executed command 'resource-stop'.

but the node process is still running and the url continues to accept traffic.

This is not occurring in the 8.2.0 Nuget Packages and SDK.

Expected Behavior

The executable ends on close or when the stop button is pressed

Exceptions (if any)

No exceptions are being thrown. Silent failure

.NET Version info

Sdk Version: 9.0.0-rc.1.24506.1
.NET 8.0.400
Windows 11 23H2

Ive tried both:

npm: 8.11.0, node: 16.16.0
npm: 10.8.2, node: 20.18.0

fails on windows.

Works on WSL.

@davidfowl davidfowl added area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication area-orchestrator labels Oct 10, 2024
@davidfowl davidfowl added this to the 9.0 milestone Oct 10, 2024
@davidfowl
Copy link
Member

Static web apps CLI? Is that what the SWA CLI is? Can you share a project that we can use to reproduce the issue?

@swissarmykirpan
Copy link
Author

swissarmykirpan commented Oct 10, 2024

Sorry, to clarify it doesn’t matter what the underlying npm task does (call swa cli or webpack), the behaviour is the same.

I’ll create a shareable project.

@swissarmykirpan
Copy link
Author

swissarmykirpan commented Oct 11, 2024

@davidfowl https://github.com/swissarmykirpan/aspire-swa, i've got two examples here, one using the swa cli and the other using another npm package called http-server. Same outcomes.

@danegsta
Copy link
Member

I think I understand what's causing this issue; npm shares its stdio with its forked children, but when we kill npm it isn't killing all the children automatically. Unfortunately we're also waiting on the stdio to finish flushing. We need to start cleaning up the children in parallel while we wait for stdio to close.

@danegsta
Copy link
Member

Yeah, this is purely an orchestrator bug; I've got a fix that can go into the next DCP insertion.

@danegsta danegsta removed the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Oct 11, 2024
@danegsta danegsta self-assigned this Oct 11, 2024
@danegsta danegsta linked a pull request Oct 14, 2024 that will close this issue
@swissarmykirpan
Copy link
Author

@davidfowl @danegsta any indication of when this will make it out to the nighty build?

@danegsta
Copy link
Member

It should already be in the latest nightly builds; looks like several CI runs have completed since the fix was merged.

@zejji
Copy link

zejji commented Oct 16, 2024

I'm still seeing this issue in 9.0.0-rc.1.24511.1

I've followed the instructions to get daily builds and can't see any more recent versions than this when I check for updated packages in Visual Studio.

I don't know whether it's relevant, but I also have other npm projects running at the same time on my computer which aren't managed by Aspire.

It would be very helpful if this situation is handled correctly as I often need to work on both a .NET Aspire project and a non-.NET Aspire project simultaneously. It will also prove to be very confusing behavior for more junior team-mates.

@danegsta
Copy link
Member

The rc.1 builds are from a separate release candidate branch that was forked before this fix was merged. Nightly builds from main should now be on 9.0.0-rc.2.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants