archimatetool / archi-modelrepository-plugin

coArchi - a plug-in to share and collaborate on Archi models.
153 stars 52 forks source link

Gitlab error message is not reported #119

Closed markbacker closed 4 years ago

markbacker commented 4 years ago

The coArchi plug-in does not report a GitLab error to the user. For users it's unclear why a 'Publish changes' command didn't work.

Used versions

coArchi version 0.5.3 with Archi 4.6.0 Build 201910010742 on Windows 10

Expected Behaviour

Publish changes results in a remote commit or an error message shows up

Actual Behaviour

No remote commit and no error message

Steps to Reproduce the Behaviour

  1. Create a repository with a 'protected branch'
    • GitLab creates default a new repository with protected master branch
    • only users with role maintainer or owner are allowed to push to master branch
  2. in Archi Edit > Preferences > Collaboration
    • use a gitlab account that doesn't have the role 'maintainer' or 'owner'
    • used account has role 'developer'
  3. Import remote model in Archi
  4. Make some changes and 'Publish changes'
    • it appears the commit is being published, but nothing happens

Using the command line and a git push there is the error "remote: GitLab: You are not allowed to push code to protected branches on this project."

... \model-repository\test (master -> origin)
λ git push https://<develop-user>:<pw>@gitlab.com/vng-realisatie/architectuur/zandbak/test.git
Enumerating objects: 19, done.
Counting objects: 100% (19/19), done.
Delta compression using up to 4 threads
Compressing objects: 100% (12/12), done.
Writing objects: 100% (15/15), 1.36 KiB | 347.00 KiB/s, done.
Total 15 (delta 6), reused 0 (delta 0)
remote: GitLab: You are not allowed to push code to protected branches on this project.
To https://gitlab.com/vng-realisatie/architectuur/zandbak/test.git
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'https://<develop-user>:<pw>@gitlab.com/vng-realisatie/architectuur/zandbak/test.git'

Suggestion

It would be very helpfull for troubleshooting if coArchi saves a log with the git commands and responses. This log-file can be opened from disk or shown in a Collaboration git-log window.

Also good for understanding what 'magic' git-commands are executed by coArchi!

Phillipus commented 4 years ago

We're using jGit for the git commands. Sometimes the errors returned by jGit are not clear, for example if may be something like "git-upload-pack not permitted...". The coArchi code does catch all exceptions and displays them to the user where possible but I'll take another look at this aspect and see if there's anything more we can do.

jbsarrodie commented 4 years ago

I face the same issue. I've just tested with coArchi 0.6 with the hope we'll now see an error been reported, but this is still silent.

This is a real UX issue as this leads people to think it was a success while it was not, and in the case of protected branch, this leads to people thinking they are allowed to do so while this is forbidden.

If really JGit doesn't tell us the real status, then at least we could check after the "Publish" or "Merge Branch" if local HEAD and remote HEAD are the same. If not, then there were an issue and we can warn the user. I would go one step further: if this happens after a merge, I would display a message which suggests that this might be related to user right issue and suggest to undo the latest merge commit.

Phillipus commented 4 years ago

@jbsarrodie Could you debug trace this and see if any useful Exception is thrown by JGit, or if it just quietly consumes it? I assume it happens on the "Push" command.

jbsarrodie commented 4 years ago

@jbsarrodie Could you debug trace this and see if any useful Exception is thrown by JGit, or if it just quietly consumes it? I assume it happens on the "Push" command.

I'll do asap. But at least I can telel you that there's nothing in Archi's error log...

Phillipus commented 4 years ago

The actual JGit Push call is surrounded by try catch(Exception) clauses in order to catch any possible JGitInternalException. This error should be caught. If we could locate where the error occurs in JGit we could see what we could do.

Or I could look at it if someone could set me up with access to a test repo where I could simulate the problem my end.

jbsarrodie commented 4 years ago

This error should be caught.

Unless there's no error at all...

Or I could look at it if someone could set me up with access to a test repo where I could simulate the problem my end.

Create a repo on GH, publish a model in it from Archi, and then go back to repo's settings on GH. Then create a rule to protect master branch: image

Then, in Archi, create a branch, publish some work on this branch (should work), then merge into master, this should silently fail.

Phillipus commented 4 years ago

then merge into master, this should silently fail.

I did "online" merge and it pushed OK. Must be some other setting needed on Github?

Phillipus commented 4 years ago

Whatever I try with that setting it allows me to push.

Edit: need to tick "Include Administrators"

Phillipus commented 4 years ago

Got it. We need to check the PushResult when pushing.

Phillipus commented 4 years ago

Here's the thing. We can get the PushResult and it has the message about not being to push in it, but we can't check that for when something goes wrong. We need to get some information from PushResult that the push did not happen and then get the message. Edit: fixed.

Phillipus commented 4 years ago

A fix has now been pushed to master. When publishing from coArchi, the returned status is checked and if not OK, the error message from JGit is displayed.

error

jbsarrodie commented 4 years ago

Not sure to fully understand your point. What you say is that you can get the message, but there is no formal status allowing you to know if things went right or wrong ?

If so, for me the "trigger" is when we did push and then after this push, local and remote HEAD are not on the same commit. This is enough to know something went wrong, and then get the message from the PushResult to show it to the user.

jbsarrodie commented 4 years ago

A fix has now been pushed to master. When publishing from coArchi, the returned status is checked and if not OK, the error message from JGit is displayed.

My GH windows was not up to date. Perfect !

Phillipus commented 4 years ago

Fixed in 0.6.2