fix:empty directory page after logged in for the first time

[jordan-ae]

Resolves #47

Here's a concise pull request description for the changes you've made:

Description:
This pull request addresses an issue in the fetchIssuePreviews function where an error thrown by checkPrivateRepoAccess would prevent the population of the freshIssues array. The error handling logic was prematurely exiting the function, resulting in an empty array being returned.

Here's a rundown of what happens in the fetchIssuePreview function

async function fetchIssuePreviews() {
  let freshIssues = [];
  try {
    // This will throw an error if the condition in checkPrivateRepoAccess's catch block is not met
    hasPrivateRepoAccess = await checkPrivateRepoAccess();
    // Code to populate freshIssues does not run, it moves directly to the catch block below...
  } catch (error) {
    // Error handling...
    // Since an error was thrown, freshIssues remains empty
  }
  // Returns tasks based on the empty freshIssues array
  return freshIssues.map(/* ... */);
}

Changes:

• Removed the throw error line in the checkPrivateRepoAccess function's catch block.
• This ensures that even if an error occurs, the fetchIssuePreviews function can continue to execute and populate freshIssues with issues from the public repository.

[jordan-ae]

Hey @0x4007 I took a lot of time out to debug #49 I posted my findings concerning the root of the bug. I'd created a PR to address it. Took a quick break before committing and when I came back fix based on my findings was attached to another issue and closed this issue as completed! Does that mean I dont get the bounty and all the hours and effort I put into this were a waste. Despite me finding the bug and providing a fix?

[rndquu]

TBH I don't fully understand what to do with this PR

Timeline:

1. @jordan-ae self assigned this issue with deadline on 8 May and missed it
2. @gentlementlegen opened a PR fixing this issue on 10 May
3. @jordan-ae opened this PR on 13 May

@gentlementlegen Does this PR solve #47 ? If #47 is fixed and this PR doesn't solve the issue then we should close this PR.

[gentlementlegen]

@rndquu This PR (#47) coincidentally solved the issue because I needed that specific part of the code the work in order to finish my task, which is why I ending up referencing it in my PR.

I think this PR should be closed without merge as another is worked on by @jordan-ae to fix a related problem. @jordan-ae please comfirm!

[jordan-ae]

@rndquu Well this PR contains a cache related fix that we'll potentially still have to do in the future. So it's up to you guys. I too am confused!

[rndquu]

@rndquu Well this PR contains a cache related fix that we'll potentially still have to do in the future. So it's up to you guys. I too am confused!

I don't understand. Does this "cache related fix" connected with #47? Or we should create a separate issue for it?

[jordan-ae]

Yes it's related to this issue

[Keyrxng]

• work.ubq.fi/src/home/fetch-github/fetch-issues-preview.ts

  Line 28 in fed9bac

  // If the status is 404, it means the user is not a collaborator, hence no access

This 404: not a collaborator line of code here should of been handling the user not being a collaborator if I'm not mistaken which was introduced months ago and the thrown error bubbled up through the try{}catch{}. Had a look over things to see if I should of been called to fix it if I broke it lmao.

The docs do state that it emits a 404 if the user is not a collaborator, and the 403 is forbidden so the issue must be authToken related, no?

[rndquu]

• work.ubq.fi/src/home/fetch-github/fetch-issues-preview.ts

  Line 28 in fed9bac

  // If the status is 404, it means the user is not a collaborator, hence no access

This 404: not a collaborator line of code here should of been handling the user not being a collaborator if I'm not mistaken which was introduced months ago and the thrown error bubbled up through the try{}catch{}. Had a look over things to see if I should of been called to fix it if I broke it lmao.

The docs do state that it emits a 404 if the user is not a collaborator, and the 403 is forbidden so the issue must be authToken related, no?

There is no 403 error in the screenshot here so the issue seems to be not authToken related

[rndquu]

@jordan-ae I can't reproduce the initial issue with empty page

This commit is meant to fix the issue while this commit should contain the issue and display empty page on the 1st login.

When I'm on 2672cbf how can I reproduce the issue with empty page? When I'm logged in for the 1st time with empty cache (i.e. local storage with saved github issues) I still see github issues while expected behaviour (based on issue description) is to get an empty page.

[Keyrxng]

but for collaborators, the line hasPrivateRepoAccess = await checkPrivateRepoAccess(); triggers a 403 error

I was basing the 403 on this comment here and that before the auth flow got changed a while back I noticed that on first render after logging in octo wasn't provided the auth token, so I forced another login to fix it. Which looked like a page reload in all honestly but I thought it seemed similar, but I could be wrong

[jordan-ae]

@jordan-ae I can't reproduce the initial issue with empty page

Hey @rndquu, the error initially wasn't easy to repro on development so I had to debug the issue directly on Production because that's where the bug was re-occuring easily.

Using Chrome DevTools, I tracked the event flow and identified an error causing the application to block collaborator issue loading. This error impacted everyone except core members, who have hasPrivateRepoAccess set to true, bypassing the problematic code.

My pull request #49 addressed the error and a caching issue that perpetuated it.

After highlighting my observations here @gentlementlegen sent in a fix for it. Wasn't sure what to do because I'd already prepared a PR for it so I sent it in anyway.

Since #48 was merged first the error was fixed.

It's a bit frustrating on my part because it took me several days to track down the bug just for my observations to be used by someone else to resolve the issue.

The fixes provided on this #49 & #48 for this issues are pretty close with #49 having an added cosmetic fix to it.

Was sending in my observations to try to get some opinions before sending in a PR. I didn't know it would end up costing me the issue. Guess I'd just have to take the L on this an let it go.

This issue can pr can be closed since a PR fixing the bug has already been provided for it. Thanks

[rndquu]

This error impacted everyone except core members

Yes, I was able to reproduce the issue. If you're logged in as a collaborator (i.e. not a core team member) then at 2672cbf you're getting an empty list of issues.

The culprit is this line.

This issue is solved by 2 PRs:

• fix: lowered user permissions on registration #48 (by adding a more strict error check)
• fix:empty directory page after logged in for the first time #49 (simply by removing error throwing)

Timeline of events:

1. @jordan-ae self assigned this issue on 7 May with deadline on 8 May and missed it
2. @gentlementlegen opened a fix: lowered user permissions on registration #48 fixing this issue on 10 May (which was later merged)
3. @jordan-ae posted a comment describing the root cause of an issue on 13 May
4. @jordan-ae opened this PR on 13 May (which is now closed)

So @gentlementlegen didn't have a change to see this comment describing the root cause of an issue.

TLDR:

• deadline was initially missed by @jordan-ae
• @gentlementlegen fixed the issue when it was unassigned + didn't get a chance to see the comment describing the root cause

I'm inclined towards closing this PR and setting @gentlementlegen as an assignee of #47 unless @0x4007 steps in.