eclipse-archived / codewind

The official repository of the Eclipse Codewind project
https://codewind.dev
Eclipse Public License 2.0
114 stars 44 forks source link

Improve "failed to restart" messages #478

Open sujeilyfonseca opened 5 years ago

sujeilyfonseca commented 5 years ago

Codewind version: 0.4.0 and Latest Development Version OS: Windows 10 and MacOS Mojave (10.14.6)

IDE extension version: 0.4.0 IDE version: VS Code 1.38.1

Description: In previous releases, there were warning messages when debug mode was not supported with an Appsody project. However, in Codewind 0.4.0, there is a request error indicating that "AppsodyExtension projects do not support restarting in debugNoInit mode".

Screen Shot 2019-09-18 at 8 50 06 AM

Steps to reproduce:

  1. Install Codewind
  2. Start Codewind
  3. Create an Appsody project without support for Debug Mode
  4. Right click on the created project
  5. Click on "Restart in Debug Mode"

@jagraj

tetchel commented 5 years ago

so, this issue is to improve the message? I'm not sure what the message may have been previously, this has always been the (not very good) message for unsupported restarts.

makandre commented 5 years ago

The previous message was Failed to attach debugger to <project>. No debug type available for project of type Extension. (see #363)

Basically old behaviour was allowing to restart in debug mode but then failing to attach debugger. Now we disallow restarting in debug mode and this happens to be the message for that

(note this is only for appsody python project which we don't have debug type for, not all appsody projects)

tetchel commented 5 years ago

ok, this is the expected behaviour then, but I agree the message sucks.

tobespc commented 5 years ago

moving to iterative-dev as that is where the message originates

rajivnathan commented 5 years ago

@sujeilyfonseca @tetchel @makandre Any suggestions for what the error message should be?

rajivnathan commented 5 years ago

I think part of the problem is that the message is constructed from 3 separate components.

  1. From the IDE: cw-appsody-python-flask: Restarting into debug mode failed:
  2. From portal: Request error for project <projectid>.
  3. From turbine: AppsodyExtension projects do not support restarting in debugNoInit mode.
makandre commented 5 years ago

IMO, I would prefer to see something like:

<project name> does not support... (something about debugging)

The problem with current message AppsodyExtension does not support... is that's not entirely accurate because some AppsodyExtension projects does support debugging

sujeilyfonseca commented 5 years ago

I agree with @makandre. As he said, it would be nice to include something like:

<project-name> does not support Debug Mode.

If possible, it would be good to display it in the IDE as a warning message instead of an error, or not provide the option of "Restart in Debug Mode".

tetchel commented 5 years ago

or not provide the option of "Restart in Debug Mode".

this is indeed hidden if the project capabilities API states that the project does not support debug, eg see any Docker type project

rajivnathan commented 5 years ago

@makandre should these project types state that they don't support debug? Could the underlying problem be a problem with the appsody project extension configs? Looking at the code it looks at whether the appsody config defines debug ports to determine whether it has the debug capability.

See https://github.com/eclipse/codewind/blob/514d1eefdc7749c67fd8a9419d340c2759cabd2f/src/pfe/file-watcher/server/src/projects/ShellExtensionProject.ts#L279

makandre commented 5 years ago

There's actually only 1 appsody project type - but depending on the appsody stack used, debug may or may not be supported (currently controlled by whether there's a debug port that maps to that stack)

rajivnathan commented 5 years ago

That sounds like the problem. We should be able to map a specific project to a specific appsody stack to accurately determine what capabilities are supported for that project. Showing the restart in debug option to the user for a stack that doesn't support it sounds like a bug.

rajivnathan commented 5 years ago

@makandre If you agree with the last comment think we can transfer the bug to appsody area? Or do you and @tetchel think the message still needs an update?

tetchel commented 5 years ago

It's super minor, but I don't think we should expose "debugNoInit" to the user, can we just call it debug mode? https://github.com/eclipse/codewind-vscode/blob/master/dev/src/codewind/project/ProjectCapabilities.ts#L39

I also think "request error for \<projectID>" is basically useless to the user.

makandre commented 5 years ago

I agree with @tetchel that this is more of a message change.

I am not sure how, or whether we should, try to map capabilities to specific stack in Codewind, because "stack" is an appsody concept (and part of the extension side) and Codewind shouldn't be required to understand what a stack is