Github: Pull request review crashes within a project of multiple git repositories

[aviatesk]

Prerequisites

• [x] Put an X between the brackets on this line if you have done all of the following:
  
  
  
  • Reproduced the problem in Safe Mode: https://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
  
  
  • Followed all applicable steps in the debugging guide: https://flight-manual.atom.io/hacking-atom/sections/debugging/
  
  
  • Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
  
  
  • Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
  
  
  • Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages
  
  

Description

First off, thanks for this Pull Request reviewing feature ! It's so nice and boosts my OSS works.
Still I found a bug within the feature thus let me report it.

The PR reviewing works fine when only a single git repository is added to a project, but once the other one more git project is added, this feature crashes. Once it gets crashed, it keeps crashes even after reloading (or removing the added repository)

(I'm on Windows 10)

Steps to Reproduce

1. Add a single git repository to a project and confirm the PR review feature works
2. Add another git repository
3. Open a PR with reviews and use the feature
4. It would crash 😢:
   
   
   
   • I can't use the other git-integrated features
   
   

The Chrome developer console emits the following error:

TypeError: Cannot read property 'diffToFilePosition' of undefined
    at ReviewsView.getTranslatedPosition (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4207865)
    at ReviewsView.e (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4196809)
    at Array.map (<anonymous>)
    at ReviewsView.renderReviewCommentThreads (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4207139)
    at ReviewsView.render (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4203393)
    at Qg (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2458467)
    at Og (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2458262)
    at Tg (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2462095)
    at bi (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2486101)
    at ci (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2486485)
    at Di (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2493375)
    at Yh (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2492755)
    at Xh (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2491776)
    at qf (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2490647)
    at Object.enqueueSetState (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2440291)
    at ObserveModel.E.setState (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2369004)
    at ModelObserver.ObserveModel._defineProperty [as didUpdate] (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:3494620)
    at ModelObserver._refreshModelData (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:3951304)
    at <anonymous>

VM22 <embedded>:14 Uncaught (in promise) TypeError: Cannot read property 'diffToFilePosition' of undefined
    at ReviewsView.getTranslatedPosition (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4207865)
    at ReviewsView.e (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4196809)
    at Array.map (<anonymous>)
    at ReviewsView.renderReviewCommentThreads (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4207139)
    at ReviewsView.render (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:4203393)
    at Qg (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2458467)
    at Og (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2458262)
    at Tg (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2462095)
    at bi (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2486101)
    at ci (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2486485)
    at Di (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2493375)
    at Yh (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2492755)
    at Xh (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2491776)
    at qf (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2490647)
    at Object.enqueueSetState (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2440291)
    at ObserveModel.E.setState (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:2369004)
    at ModelObserver.ObserveModel._defineProperty [as didUpdate] (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:3494620)
    at ModelObserver._refreshModelData (C:\Users\aviat\AppData\Local\atom\app-1.37.0\resources\app\static\<embedded>:14:3951304)
    at <anonymous>
getTranslatedPosition @ VM22 <embedded>:14
ReviewsView.e @ VM22 <embedded>:14
renderReviewCommentThreads @ VM22 <embedded>:14
render @ VM22 <embedded>:14
Qg @ VM22 <embedded>:14
Og @ VM22 <embedded>:14
Tg @ VM22 <embedded>:14
bi @ VM22 <embedded>:14
ci @ VM22 <embedded>:14
Di @ VM22 <embedded>:14
Yh @ VM22 <embedded>:14
Xh @ VM22 <embedded>:14
qf @ VM22 <embedded>:14
enqueueSetState @ VM22 <embedded>:14
E.setState @ VM22 <embedded>:14
ObserveModel._defineProperty @ VM22 <embedded>:14
_refreshModelData @ VM22 <embedded>:14
async function (async)
_refreshModelData @ VM22 <embedded>:14
refreshModelData @ VM22 <embedded>:14
setActiveModel @ VM22 <embedded>:14
componentDidMount @ VM22 <embedded>:14
Vh @ VM22 <embedded>:14
Zh @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
e.unstable_runWithPriority @ VM22 <embedded>:14
Fi @ VM22 <embedded>:14
Di @ VM22 <embedded>:14
Yh @ VM22 <embedded>:14
Xh @ VM22 <embedded>:14
qf @ VM22 <embedded>:14
enqueueSetState @ VM22 <embedded>:14
E.setState @ VM22 <embedded>:14
o.handleDataChange @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
_onQueryDataAvailable @ VM22 <embedded>:14
next @ VM22 <embedded>:14
next @ VM22 <embedded>:14
next @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
next @ VM22 <embedded>:14
_next @ VM22 <embedded>:14
next @ VM22 <embedded>:14
next @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
Promise resolved (async)
(anonymous) @ VM22 <embedded>:14
_subscribe @ VM22 <embedded>:14
subscribe @ VM22 <embedded>:14
Executor @ VM22 <embedded>:14
execute @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
_subscribe @ VM22 <embedded>:14
subscribe @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
_subscribe @ VM22 <embedded>:14
subscribe @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
_subscribe @ VM22 <embedded>:14
subscribe @ VM22 <embedded>:14
fetch @ VM22 <embedded>:14
fetchQueryAndComputeStateFromProps @ VM22 <embedded>:14
ReactRelayQueryRenderer @ VM22 <embedded>:14
vf @ VM22 <embedded>:14
Og @ VM22 <embedded>:14
Tg @ VM22 <embedded>:14
bi @ VM22 <embedded>:14
ci @ VM22 <embedded>:14
Di @ VM22 <embedded>:14
Yh @ VM22 <embedded>:14
Xh @ VM22 <embedded>:14
qf @ VM22 <embedded>:14
enqueueSetState @ VM22 <embedded>:14
E.setState @ VM22 <embedded>:14
ObserveModel._defineProperty @ VM22 <embedded>:14
_refreshModelData @ VM22 <embedded>:14
async function (async)
_refreshModelData @ VM22 <embedded>:14
refreshModelData @ VM22 <embedded>:14
setActiveModel @ VM22 <embedded>:14
componentDidMount @ VM22 <embedded>:14
Vh @ VM22 <embedded>:14
Zh @ VM22 <embedded>:14
(anonymous) @ VM22 <embedded>:14
e.unstable_runWithPriority @ VM22 <embedded>:14
Fi @ VM22 <embedded>:14
Di @ VM22 <embedded>:14
Yh @ VM22 <embedded>:14
Ii @ VM22 <embedded>:14
Cd @ VM22 <embedded>:14

Reproduces how often:

This crash happens for every project containing multiple git repositories

Note

Once this feature crashes for that repository, this feature never works in for the repo even after reloading and the other temporal remedies.

Versions

atom --version

Atom    : 1.37.0
Electron: 2.0.18
Chrome  : 61.0.3163.100
Node    : 8.9.3

apm --version

apm  2.1.7
npm  6.2.0
node 8.9.3 x64
atom 1.37.0
python 3.7.1
git 2.21.0.windows.1
visual studio

---

Edit: I've just confirmed the same crash after updating my Atom to v1.38.0 ...

[wadethestealth]

@aviatesk Can confirm I have the exact same behavior. For my setup, I have 3 remotes (repository from nteract, my fork of that, and aviatesk's fork). When I first open atom up I also see

https://api.github.com/repos/wadethestealth/hydrogen/pulls/1683
Failed to load resource: the server responded with a status of 404 (Not Found)

It should be https://api.github.com/repos/nteract/hydrogen/pulls/1683 not my username.

When I click see reviews I then get the same error as aviatesk.

[rsese]

Thanks for the report @aviatesk, a couple of questions:

• Add a single git repository to a project and confirm the PR review feature works
• Add another git repository

Just to make sure I understand, can you clarify what you mean by the second step? At first I thought you meant open 2 different projects in the same Atom window but I don't think that's what you mean.

• Open a PR with reviews and use the feature

When you say "use the feature", is it enough to just open the reviews for a pull request? Or do you need to do something like leave a comment?

[smashwilson]

From your stack trace, @aviatesk, I'm guessing that it has to do with the patch that's loaded from a PR associated with the active branch in your second repository. I recently fixed a bug that _might_ share a root cause with yours... can you try to repro in Atom 1.40.0-beta0 and see if it still occurs?

@wadethestealth, the 404 looks like a different error related to us not understanding forks properly. Would you mind filing that as a separate issue?

[wadethestealth]

Sure @smashwilson

[aviatesk]

@rsese
Sure I will elaborate the repro steps:

1. Add a single git repository to a project and confirm the PR review feature works
2. Add another git repository

Just to make sure I understand, can you clarify what you mean by the second step? At first I thought you meant open 2 different projects in the same Atom window but I don't think that's what you mean.

Let me rewrite them like below:

1. Add a git project to the current Atom window (like via Files -> Add Project Foler...)
2. Add another separate git project to the Atom window (Meaning doing the same thing as step.1 for the different git project)
   So after the end of step.2, we have 2 different git projects in the same Atom window. The resulting image would be:
   

1. Open a PR with reviews and use the feature

When you say "use the feature", is it enough to just open the reviews for a pull request? Or do you need to do something like leave a comment?

I meant just trying to open the reviews for a PR. For my case, just trying to open Review #XXXX breaks the whole git/github integration.

[aviatesk]

@smashwilson
I played with v1.41.0-nightly0.
And found that your patch _partly_ solves this problem.

I mean, I found I can open and use this feature for one PR that previously causes the crash, but at the same time, I found the crash still happens for another PR (in the same repository).

(I can open Reviews #XXXX pane for the first PR , but if I try to open that for the second PR, the whole git/github integration breaks)

[aviatesk]

@smashwilson @rsese
Anyone working on this ?
This happens almost everytime I try to use this feature (and break all the git integration in the Atom process), and is very annoying IMHO ...

[tristan-cunningham]

Also getting the same problem on multiple repositories, though works on one or two branches.

Atom: 1.41.0
Electron: 4.2.7
Chrome: 69.0.3497.128
node: 10.11.0
Github: 0.30.1

[wadethestealth]

@rsese @smashwilson I think this was actually the case of #2302 and I didn't realize. This should already be patched by #2305.

@tristan-cunningham I don't believe the fix is published to an atom release yet (even nightly).

[aviatesk]

I really appreciate the fix, thanks so much, @wadethestealth 👍