feat: handle when starting debug session failed #1809
Conversation
kittaakos
added
type: enhancement
|
|
| async tempBuildPath(sketch: Sketch): Promise<string> { | ||
| const sketchPath = FileUri.fsPath(sketch.uri); | ||
| const hash = crypto | ||
| .createHash('md5') | ||
| .update(sketchPath) | ||
| .digest('hex') | ||
| .toUpperCase(); | ||
| return join(this.isTempSketch.tempDirRealpath, `arduino-sketch-${hash}`); |
There was a problem hiding this comment.
When I follow the reproduction instructions from #808 (comment) on my Windows machine, I get the raw error from Arduino CLI in the notification:
Error getting Debug info: Compiled sketch not found in C:\Users\per\AppData\Local\Temp\arduino-sketch-AEA9FCC61D66FCE9E9A157F96547FFB0
Instead of the expected user friendly interpretation from Arduino IDE:
Sketch 'sketch_jan16b' must be verified before starting a debug session. Please verify the sketch and start debugging again. Do you want to verify the sketch now?
I set a breakpoint here:
I see that the value of tempBuildPath is:
c:\\Users\\per\\AppData\\Local\\Temp\\arduino-sketch-956C36A70086EB62B81F7E1AA8AEE643
In addition to the difference in hash, I notice the different case of the drive letters (c vs C).
I get the expected user friendly notification text on my Linux machine.
There was a problem hiding this comment.
Thanks for the review. I patched the PR. It's a complete hack, but finally, it works on Windows too:
808_win.mp4
macOS:
808_macos.mp4
| .update(path) | ||
| .digest('hex') | ||
| .toUpperCase(); | ||
| const folderName = `arduino-sketch-${hash}`; |
There was a problem hiding this comment.
This is correct for the version of Arduino CLI currently used by Arduino IDE, but it will break when the Arduino CLI dependency is bumped:
- Store all temporary files under a single folder arduino-cli#2031
- feat: purge build cache arduino-cli#2033
After those changes, the path is now <temp dir>/arduino/sketches/<hash>.
No action needed now, but I thought to add a note in anticipation of the adjustment that will be needed at the time of the Arduino CLI dependency bump.
There was a problem hiding this comment.
Thank you for the heads-up!
There was a problem hiding this comment.
I pushed a test for the build path: fd38ba4.
I had to change the IDE2 logic when stopping and restarting the daemon. I have to double-check it.
There was a problem hiding this comment.
I have to double-check it.
The ready deferred promise is not used anywhere else; the change must not break anything. We need to clean up this code next time we change the core gRPC clients.
There was a problem hiding this comment.
I have decided to drop everything I changed after the review and moved it to a separate PR: #1823. If the build is green, I would like to merge it as it is if you are OK with it, Per. Thanks!
There was a problem hiding this comment.
Yes, I'm OK with that. Feel free to merge anytime you are ready Akos.
If the sketch has not been verified, IDE2 offers user a verify action. Closes #808 Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
IDE2 does not know what casing the CLI uses Signed-off-by: Akos Kitta <a.kitta@arduino.cc>
Motivation
IDE2 should clearly communicate to the user when starting a debug session has failed due to an unverified sketch. In this case, users have to verify the sketch and restart the debug session.
Change description
This PR consists of two changes:
808.mp4
Steps to verify:
yesaction to verify the sketch,Other information
Reviewer checklist
Closes #808