reconquest / atlassian-external-hooks

External Hooks plugin for Atlassian Bitbucket
https://external-hooks.reconquest.io
Other
44 stars 37 forks source link

"Can't get user email address" when receiving packets from "system accounts" #60

Closed chburmeister closed 4 years ago

chburmeister commented 6 years ago

While monitoring our Bitbucket logs, we are facing a problem when the plugin tries to get the email adress of a "system account" that has only an access key to interact with Bitbucket:

/src/main/java/com/ngs/stash/externalhooks/hook/ExternalPreReceiveHook.java:

if (currentUser.getEmailAddress() != null) {
   env.put("STASH_USER_EMAIL", currentUser.getEmailAddress());
} else {
   log.error("Can't get user email address. getEmailAddress() call returns null");
}

OK, it cannot find an email adress as the committer is no real user with name/adress but is this really worth an error-loglevel? Is this STASH_USER_EMAIL information somewhere needed where it could be better logged as absent? By this logging, we got a log of false-positives.

urol commented 5 years ago

Bitbucket: 6.4.0 external-hooks-6.3.1-1

Its just a WARN, but is it worth to print the stacktrace. It fills up the log approx 200 times a day in our case. The env var seems to be missing at this point in the case the user/mail comes from google groups via a UCS system.


--
java.lang.NullPointerException: null value in entry: BB_USER_EMAIL=null
at com.google.common.collect.CollectPreconditions.checkEntryNotNull(CollectPreconditions.java:32)
at com.google.common.collect.ImmutableMap.entryOf(ImmutableMap.java:176)
at com.google.common.collect.ImmutableMap$Builder.put(ImmutableMap.java:285)
at com.atlassian.stash.internal.hook.script.DefaultHookScriptEnvironmentProvider.userDetails(DefaultHookScriptEnvironmentProvider.java:203)
at com.atlassian.stash.internal.hook.script.DefaultHookScriptEnvironmentProvider.create(DefaultHookScriptEnvironmentProvider.java:115)
at com.atlassian.stash.internal.hook.script.DefaultHookScriptInvoker.lambda$prepareEnvironment$1(DefaultHookScriptInvoker.java:286)
at java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
at com.atlassian.stash.internal.hook.script.DefaultHookScriptInvoker.prepareEnvironment(DefaultHookScriptInvoker.java:287)
at com.atlassian.stash.internal.hook.script.DefaultHookScriptInvoker.postUpdate(DefaultHookScriptInvoker.java:114)
at com.atlassian.stash.internal.plugin.OsgiSafeProxyProvider$1.invoke(OsgiSafeProxyProvider.java:95)
at com.atlassian.stash.internal.hook.script.ScriptRepositoryHook.postUpdate(ScriptRepositoryHook.java:35)
at com.atlassian.stash.internal.hook.repository.DefaultRepositoryHookService.postUpdate(DefaultRepositoryHookService.java:760)
at com.atlassian.stash.internal.hook.repository.DefaultRepositoryHookService.lambda$doPostUpdate$7(DefaultRepositoryHookService.java:542)
at org.springframework.transaction.support.TransactionTemplate.execute(TransactionTemplate.java:140)
at com.atlassian.stash.internal.hook.repository.DefaultRepositoryHookService.doPostUpdate(DefaultRepositoryHookService.java:526)
at com.atlassian.stash.internal.hook.repository.DefaultRepositoryHookService.postUpdateSynchronous(DefaultRepositoryHookService.java:336)
at com.atlassian.stash.internal.hook.DefaultBuiltInHookHandlerFactory.lambda$postReceive$1(DefaultBuiltInHookHandlerFactory.java:69)
at com.atlassian.stash.internal.hook.DefaultHookService.doHandleRequest(DefaultHookService.java:301)
at com.atlassian.stash.internal.hook.DefaultHookService.handleRequest(DefaultHookService.java:287)
at com.atlassian.stash.internal.hook.DefaultHookService.handleRawRequest(DefaultHookService.java:221)
at com.atlassian.stash.internal.hook.DefaultHookService$1.lambda$run$0(DefaultHookService.java:188)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
at java.lang.Thread.run(Thread.java:748)
... 34 frames trimmed```
seletskiy commented 5 years ago

@urol ~thanks for the bug report. It will be fixed soon~.

Actually, I've read stacktrace more carefully and it happens to be a bug in BB itself, since it does not even reach code in our plugin.

@bturner: does that sounds like a bug to you? It seems BB code doesn't set/check BB_USER_EMAIL on hook script trigger.

bturner commented 5 years ago

There seems to be 2 different manifestations of a similar issue under discussion here. The original description, from @chburmeister, is definitely not Bitbucket Server code. @urol’s error message is Bitbucket Server, though, definitely. Since e-mail addresses aren’t required (for example when Bitbucket Server is connected to an LDAP server), I agree there’s an issue there. For that case the hooks will completely fail to run; it’s not a “trivial” unnecessary warning message like the one from the description.

bturner commented 5 years ago

A fix for the Bitbucket Server issue is tracked at BSERV-11869.

seletskiy commented 5 years ago

@bturner Thank you, Bryan!

kovetskiy commented 4 years ago

@bturner Hi, Bryan

I see that BSERV-11869 is marked as closed and your comment that the patch was merged.

Could you let us know in which version of Atlassian Bitbucket the patch was released?

Is it 6.5.1?

bturner commented 4 years ago

@kovetskiy At the time I filled in the fix versions, only 6.5.1 had been released. I backported it further, though. Hook script support shipped in 6.2.0, so I backported it to our release/6.2 branch. Since the issue was closed, new point releases have been cut. I've updated the issue to show the full set of fix versions:

Hope this helps, Bryan Turner Atlassian Bitbucket