-
Notifications
You must be signed in to change notification settings - Fork 11
fix: handle quarkus launch failure #5677
Conversation
matt2e
commented
Jun 4, 2025
- Previously if quarkus failed to launch the build would be stuck for 100 mins waiting to connect
- Now it returns with the actual build errors
| logger.Infof("Dev mode process exited") | ||
| cancel(errors.Wrap(context.Canceled, "dev mode process exited")) | ||
| } | ||
| func (s *Service) launchQuarkusProcess(ctx context.Context, devModeBuild string, projectConfig projectconfig.Config, buildCtx buildContext, stdout *errorDetector, errChan chan error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing in the errChan here so that failure to launch is passed back
| BuildFailure: &langpb.BuildFailure{ | ||
| Errors: ers, | ||
| }}}), nil | ||
| case err := <-errorChan: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errChan now gets either the failure to launch error, or the error from the above go func() {} (eg connect timeout), whichever fails first
| return nil, errors.WithStack(err) | ||
| } | ||
|
|
||
| release, err := flock.Acquire(ctx, buildCtx.Config.BuildLock, BuildLockTimeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
build lock is now locked on each build, not just the first build in runQuarkusDev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we even need the build lock now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it still prevents two ftl build s running at once I think. But I think it should be moved out of the plugins now. I'll do that in a separate PR
| } | ||
|
|
||
| func (s *Service) launchQuarkusProcessAsync(ctx context.Context, devModeBuild string, projectConfig projectconfig.Config, buildCtx buildContext, stdout *errorDetector) { | ||
| go func() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moved the go keyword to the actual function call which I think is more idiomatic
825e401 to
b8d3feb
Compare