jcgay / maven-notifier

Desktop notifications for Maven 3.x.
Other
76 stars 12 forks source link

fails under mvnd #108

Closed delanym closed 2 years ago

delanym commented 2 years ago

The notifier fails to notify after the reactor builds finishes successfully when running under https://github.com/mvndaemon/mvnd

[WARN] Failed to notify spy fr.jcgay.maven.notifier.NotificationEventSpyChooser: Error while sending notification to notify-send.
Check your configuration at: https://github.com/jcgay/send-notification/wiki/notify-send
jcgay commented 2 years ago

Does it works with the classic mvn ?

I'm not sure what is really going on under the hood of mvnd but has a mvn is always up in the background, I don't know if the org.apache.maven.eventspy.EventSpy contract is honored 🤷‍♂️.

I have just try it, the notification is fine for me when I start a fresh daemon, but additional builds on the same daemon do not trigger a notification 😅.

I will make more research 😇.

delanym commented 2 years ago

@jcgay yes it works with mvn 3.6.3 and mvnw 3.8.1

jcgay commented 2 years ago

I have managed to debug the mvnd daemon with mvnd -Dmvnd.debug=true, the spy is correctly called (init, onEvent and close) but the notification is not displayed at the second (and others) build on the same daemon.

I will dig a little more into it later, I may have a strange state in maven-notifier which breaks :(

jcgay commented 2 years ago

Ok it's definitely a problem from maven-notifier, not mvnd, I do initialization in constructors but they are not re-instantiate between builds with mvnd whereas mvn do not hold state as a JVM is restart for each build.

I'll have to rewrite some stuff to make it run with mvnd 😇.

delanym commented 2 years ago

Great work - I've tested the snapshot

jcgay commented 2 years ago

Cool 👍 I will drop a release soon :)

delanym commented 2 years ago

Hmmm, it was working, but now i just got this

[WARN] Failed to notify spy fr.jcgay.maven.notifier.NotificationEventSpyChooser: Error while sending notification to notify-send. Check your configuration at: https://github.com/jcgay/send-notification/wiki/notify-send

Because its a warning can't add -e and provide a stacktrace. Could you add a config to fail the build if it doesn't notify?

jcgay commented 2 years ago

Oh, It's seem still fine for me (but using notification-center or growl on macOS.
Let me try something else anyway, it's not fully functional with the first fix, we can't change the configuration between run on the same daemon 😅

jcgay commented 2 years ago

The WARN log comes from mvn itself, you should be able to get the stacktrace by running mvn in debug mode (-X,--debug).

In org.apache.maven.eventspy.internal.EventSpyDispatcher#logError:

private void logError( String action, Throwable e, EventSpy spy )
    {
        String msg = "Failed to " + action + " spy " + spy.getClass().getName() + ": " + e.getMessage();

        if ( logger.isDebugEnabled() )
        {
            logger.warn( msg, e );
        }
        else
        {
            logger.warn( msg );
        }
    }