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

Generate webpage in case of exceptions #93

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fahad-israr
Copy link
Collaborator

@fahad-israr
Copy link
Collaborator Author

  • This works well in dev mode but when running the generated Native Binary I face the following error:
java.awt.HeadlessException
	at java.awt.Desktop.getDesktop(Desktop.java:301)
	at dev.starfix.Starfix.generateHTML(Starfix.java:372)
	at dev.starfix.Starfix.cloneCmd(Starfix.java:68)
	at dev.starfix.Starfix.run(Starfix.java:111)
	at picocli.CommandLine.executeUserObject(CommandLine.java:1939)
	at picocli.CommandLine.access$1300(CommandLine.java:145)
	at picocli.CommandLine$RunLast.executeUserObjectOfLastSubcommandWithSameParent(CommandLine.java:2352)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2346)
	at picocli.CommandLine$RunLast.handle(CommandLine.java:2311)
	at picocli.CommandLine$AbstractParseResultHandler.execute(CommandLine.java:2179)
	at io.quarkus.picocli.runtime.PicocliRunner$EventExecutionStrategy.execute(PicocliRunner.java:26)
	at picocli.CommandLine.execute(CommandLine.java:2078)
	at io.quarkus.picocli.runtime.PicocliRunner.run(PicocliRunner.java:39)
	at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:123)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:66)
	at io.quarkus.runtime.Quarkus.run(Quarkus.java:42)
	at io.quarkus.runner.GeneratedMain.main(GeneratedMain.zig:30)
  • One of the solution I can think is to use out command runner(ProcessExecutor) and based on OS. Example we can use open https://link on macosx . Similarly for Linux we can use xdg-open https://link and for Windows: start https://link

@maxandersen
Copy link
Member

yeah looks like desktop not available in native yet.

having custom launch implmeented probably best approach if headless exception thrown

@fahad-israr
Copy link
Collaborator Author

Have added OS Specific ways to open up webpage : fahad-israr@4269882

@@ -8,6 +8,8 @@

package dev.starfix;
import io.quarkus.runtime.annotations.RegisterForReflection;
import jdk.jfr.StackTrace;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

@@ -341,6 +360,44 @@ public static void processCloneURL(CloneUrl cloneUrl) throws URISyntaxException,

}

public static void generateHTML(StackTraceElement[] stacktrace, String message)throws IOException, InterruptedException{
String userHome = System.getProperty("user.home");
File f = new File(userHome+"/starfixException.html");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would cause conflicts if concurrent runs.
shuold also put it in temporary directory.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have made it a temp file now and delete it on exit .

throw new IllegalArgumentException("Not a valid URI for git repository");
String message = "Not a valid URI for git repository: "+cloneUrl.url;
Exception illegalArgumentException = new IllegalArgumentException(message);
generateHTML(illegalArgumentException.getStackTrace(),message);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think better to just pass in the exception then you can also pass message and other context in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes 👍
have changed it now.

@fahad-israr
Copy link
Collaborator Author

@maxandersen This PR is ready for a review.

cloneCmd(uri);
}catch(Exception e){
e.printStackTrace();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never ok to just print out stacktrace in case of error. need to handle it or guide the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants