Conversation
- Create IFacade interface defining common API for SocketIO and SignalR - Update SocketIoFacade to implement IFacade - Update SignalRFacade to implement IFacade (add Connect no-op) - Update RuntimeControllerBase.Socket to return IFacade - Update RuntimeControllerAspNetBase.Socket to return IFacade - Update RuntimeControllerDotNetFirst.Socket to return IFacade - Update ElectronNetRuntime.GetSocket() to return IFacade - Update BridgeConnector.Socket to return IFacade This enables polymorphic usage of both facades throughout the codebase and prepares for full Electron API integration with SignalR mode.
- Update signalr-bridge.js to handle .NET→Electron events via 'event' channel - Add socket.io-compatible .on() and .emit() methods to SignalRBridge - Update main.js to load all Electron API modules with SignalR bridge - Update SignalRFacade.Emit() to send events via 'event' channel - Add ElectronHub.ElectronEvent() to receive Electron→.NET events - Add SignalRFacade.TriggerEvent() to invoke .NET event handlers - Remove duplicate ElectronEvent method from hub This enables full bidirectional communication: - .NET can call Electron APIs via Emit (e.g., createBrowserWindow) - Electron can send events back to .NET (e.g., BrowserWindowCreated) - Event handlers registered via On/Once now work with SignalR
- Add RunReadyCallback execution when SignalR connects - This triggers window creation and other app initialization - Fixes missing variables in main.js (desktopCapturer, electronHostHook, touchBar, shellApi) - Window now opens successfully in SignalR mode Known issue: EPIPE console error when logging to closed pipe (to be fixed)
- Wrap all console.log/error calls in try-catch to handle EPIPE - Disable SignalR client logging (LogLevel.None) - Prevents Electron crash when console pipes are closed This fixes the 'EPIPE: broken pipe, write' error that was preventing the Electron window from displaying.
- Add WebSockets middleware to ASP.NET pipeline - Move HTTPS redirect to production only - Add extensive debug logging in startSignalRApiBridge - Enable Warning level logging in SignalR client Issue: signalRBridge.connect() hangs and never resolves - Connection starts but never completes - No error messages from SignalR client - ASP.NET shuts down after timeout - ElectronHub.OnConnectedAsync never called Next: Investigate why WebSocket connection doesn't establish
ROOT CAUSE: Electron quits when app.on('ready') completes with 0 windows.
In SignalR mode, no windows are created immediately, so Electron exits,
triggering ElectronProcess_Stopped and shutting down ASP.NET.
SOLUTION: Create an invisible 1x1 keep-alive window in SignalR mode to
prevent Electron from quitting while waiting for SignalR connection.
Also:
- Make app.on('ready') async and await startSignalRApiBridge()
- Add window-all-closed handler for SignalR mode
- Add extensive debug logging to track lifecycle
- Don't subscribe to electronProcess.Ready in SignalR controller
This fixes the premature shutdown that prevented SignalR connection.
- Fix duplicate SignalR connection in main.js (removed redundant connect block) - Fix race condition: Electron now signals 'electron-host-ready' after loading API modules - Fix type conversion in SignalRFacade for event handlers (handle JsonElement and numeric types) - Fix SignalR argument mismatch: pass args as array instead of spread, use object[] instead of params - .NET now waits for electron-host-ready before calling app ready callback
- Changed event handler to receive args as single array parameter - Spread array elements as individual arguments to match Socket.IO behavior
- Destroy keep-alive window when first real window is created (enables proper window-all-closed behavior) - Update ElectronNetRuntime.AspNetWebPort with actual port after Kestrel starts (fixes http://localhost:0 issue)
- Fix middleware order: UseAntiforgery must be between UseRouting and UseEndpoints - Add UseStaticFiles() to serve wwwroot content - Fix scoped CSS bundle reference: use lowercase 'electronnet-samples-blazorsignalr.styles.css' to match generated asset name - Add HTTP request logging for debugging - Enable detailed logging for routing and static files in development
Remove excessive console logging that was added during debugging: - Removed verbose logging from Program.cs (app ready callback steps) - Removed HTTP request logging middleware - Cleaned up RuntimeControllerAspNetDotnetFirstSignalR lifecycle logging - Streamlined ElectronHub connection/event logging - Simplified SignalRFacade event handling logging - Reduced JavaScript logging in main.js and signalr-bridge.js - Reset log levels to Warning for SignalR components in appsettings Kept only essential error logging and critical state transitions. Production-ready logging levels maintained.
Added comprehensive code comments explaining: - RuntimeControllerAspNetDotnetFirstSignalR: .NET-first startup flow and key differences from Socket.IO - SignalRFacade: Type conversion handling and event propagation details - signalr-bridge.js: Socket.IO compatibility layer and arg handling - main.js: Keep-alive window pattern and SignalR startup sequence Comments focus on explaining WHY decisions were made, not just WHAT the code does.
I'm also contributing to FFmpeg (member of the GA) and when you do similar there, you may get responses where you might just want to turn around and never come back :-) Apologies accepted though.
Let me be honest with you: My intent and motivation has always been very clear as well. I'm not a hobbyist developer who is contributing to an open-source project for fun. I made that contribution for Electron.NET Core out of strategical needs. For once, I needed to be able to debug with little delay and to work and debug a Linux application from within Visual Studio on Windows. Also Electron.NET was about to die silently and slowly and it needed some boost in order to have a future (luckily, the changes are able to achieve all at once) - because nobody wants to go all in for a dead horse. At the bottom line, there's no reason for me to deal with SignalR support in Electron.NET - it makes everything just more difficult and I need to take care that there won't be any changes that would regress our usage of it - so for me it's better when this doesn't get into Electron.NET. So,, as far as I'm concerned, I'm strongly leaning towards the "no"-side regarding SignalR integration. |
|
Thanks for the info @epsitec - I am still not sure why you insist on SignalR. From your perspective there should not be a difference. Sure, you will use SignalR in your code (which is fine) - but whatever the other connection (that you don't own nor can attach to) uses should be irrelevant. As socketio will be part of the application anyway why have two libs? Sure I can understand your position that ASP.NET Core / Blazor anyway uses it, so if socketio would not be there it would be just this, but then again it would be strongly tied to ASP.NET Core. So my position is that we can progress on this one (i.e., I will continue on it) by removing SignalR and have the secure connection done via socketio - same principles would apply here, just more simplified code imho. |
|
@FlorianRappl this might be caused by my misunderstanding of how
|
|
|
I am still convinced that our approach with a single port allocated by Kestrel and filtered using a dedicated middleware is the only realistic solution. |
…into feature/secure-connection
Release 0.4.1
…to feature/secure-connection
…onNET/Electron.NET into feature/secure-connection
pr-comment: Run #53
🎉 All tests passed!Github Test Reporter by CTRF 💚 🔄 This comment has been updated |
|
I think we are approaching the end here.
Right now I kept the Blazor sample but I can also remove it / put it in a dedicated PR. As this uses the new middleware I think keeping it in here (or otherwise making it a follow-up PR) is suitable. |
softworkz
left a comment
softworkz
left a comment
There was a problem hiding this comment.
I haven't looked at everything yet (e.g. not the auth stuff)
src/ElectronNET.API/Runtime/Services/ElectronProcess/ElectronProcessActive.cs
Show resolved
Hide resolved
|
The commit history is not sane. Maybe it's better to start from a fresh develop branch and cleanly apply the changes one after another, like:
That would also avoid stray changes (which still exist, but much better than before). Regarding the port exchange, why does it need to use parsing command line output? |
That is unfortunately not a good approach. We want to have a stable port selection - and the best approach is to just let the OS decide the port. To avoid race conditions we should pick
I agree on this one. I'd actually separate this into multiple PRs:
|
Yes, that's what I mean:
|
|
Overall, I think the auth, the ISocketConnection and better port handling are all good and valuable additions! |
3 participants
This PR was done by third-party developers; I am just here to provide their changes for review.
The basic idea is outlined in the Electron.NET SignalR Authentication Guide.
In here, the system makes sure to use exclusively respond to a request when a cookie based on an initial auth token was provided. This way, even though the application behind is open to all users of the same machine, only the "correct" one is answering.
The background for this is that sometimes the desktop application is used by connecting to a remote machine effectively creating a user session on the machine. If this is done multiple times by different users then the same application is launched multiple times on this machine. In general, multi-instance mode works fine, but has of course some limitations such as requiring dedicated ports for each running app instance.