Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle missing primaryEmail for enterprise users #2665

Merged
merged 6 commits into from
Jan 10, 2024

Conversation

abeatrix
Copy link
Contributor

CLOSE #2663 & #2664

A bug was introduced in #2233 where we added a requirement for primaryEmail during auth, resulting in uncaught error that stopped the extension from starting.

For Enterprise instances, emails are not always required to set up an account.

This PR includes the following changes to fix the issue above:

  • update authStatus and graphQL query to reflect that primaryEmail and displayName fields are optional
  • add tests for the above change
  • use username instead of displayName for enterprise instances as username is a required field
  • replace replace getEnterpriseCurrentUserInfo and getDotComCurrentUserInfo with the new getCurrentUserInfo
  • update agent tests with the changes above

Test plan

  1. Log into Cody using an enterprise soucegraph (e.g. S2) token associated with a user that doesn't have emails set up for their account.
  2. Try reloading VS Code

Before

Unable to start Cody on reload

Screen.Recording.2024-01-10.at.7.48.40.AM.mov

After

https://www.loom.com/share/b9049f54cb0f48da87fc630c84cc736e?sid=0e1b3677-9c3e-4c28-84b5-846f95ad4a16

fix.auth.error.mp4

@abeatrix abeatrix requested a review from a team January 10, 2024 17:58
@abeatrix abeatrix changed the title Bee/fix user email fix: handle missing primaryEmail for enterprise users Jan 10, 2024
@abeatrix abeatrix requested a review from a team January 10, 2024 18:06
Copy link
Contributor

@kalanchan kalanchan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ggogoog!

@abeatrix abeatrix merged commit ac67893 into main Jan 10, 2024
16 checks passed
@abeatrix abeatrix deleted the bee/fix-user-email branch January 10, 2024 19:37
arafatkatze added a commit that referenced this pull request Jan 11, 2024
Issue: #2669

The current build of Cody has an issue where the cursor is stolen many
times because of a rerendering of some sort for auth, this makes cody
very hard to use for anything as you can barely type.

Original PR that introduced this
#2665

I actually do not know how the PR works so I added a throttle mechanism
to stop the rerendering for 5 minutes(300 seconds) so that it doesn't
make a super terrible UX.

My aim is to merge this HACK for now and then do a proper fix later with
@abeatrix



## Test plan

<!-- Required. See
https://sourcegraph.com/docs/dev/background-information/testing_principles.
-->
sqs added a commit that referenced this pull request Jan 12, 2024
In #2665 we migrated the chat history key to use the user's username instead of their email address. This means that the storage would retain the chat history under the old key indefinitely.

Large storage data slows down extension host activation and every `set` operation. My storage was 1.8MB, which was causing warnings from VS Code like this:

```
[mainThreadStorage] large extension state detected (extensionId: sourcegraph.cody-ai, global: true): 916.8603515625kb. Consider to use 'storageUri' or 'globalStorageUri' to store this data on disk instead.
```

Each `Memento#set` call was taking ~37ms (with 1.8MB of data). With ~1kb of data, each set call takes ~1-3ms.

Of course, the data will accumulate as the user uses Cody more, but at least we can manage the data now without some of it being "orphaned".

Test plan:

Confirmed that this removes the old data:

1. Added a console.log to ensure that the code had the right before/after values.
2. Confirmed the storage DB on disk had the keys before and after the migration did not have them, with `sqlite3 ~/.config/Code/User/globalStorage/state.vscdb "select value from itemtable where key='sourcegraph.cody-ai';" | jq '."cody-local-chatHistory-v2" | keys'`.
3. Confirmed the length of the key's value went down (from 1.8MB to 672 bytes, as expected).
sqs added a commit that referenced this pull request Jan 12, 2024
In #2665 we migrated the chat
history key to use the user's username instead of their email address.
This means that the storage would retain the chat history under the old
key indefinitely.

Large storage data slows down extension host activation and every `set`
operation. My storage was 1.8MB, which was causing warnings from VS Code
like this:

```
[mainThreadStorage] large extension state detected (extensionId: sourcegraph.cody-ai, global: true): 916.8603515625kb. Consider to use 'storageUri' or 'globalStorageUri' to store this data on disk instead.
```

Each `Memento#set` call was taking ~37ms (with 1.8MB of data). With ~1kb
of data, each set call takes ~1-3ms.

Of course, the data will accumulate as the user uses Cody more, but at
least we can manage the data now without some of it being "orphaned".

## Test plan:

Confirmed that this removes the old data:

1. Added a console.log to ensure that the code had the right
before/after values.
2. Confirmed the storage DB on disk had the keys before and after the
migration did not have them, with `sqlite3
~/.config/Code/User/globalStorage/state.vscdb "select value from
itemtable where key='sourcegraph.cody-ai';" | jq
'."cody-local-chatHistory-v2" | keys'`.
3. Confirmed the length of the key's value went down (from 1.8MB to 672
bytes, as expected).
sqs added a commit that referenced this pull request Mar 5, 2024
The migrations in #2473, #2695, and #2665 have been in the code for one major release and more than a month. We can remove them to simplify the code now.

Also remove other unused code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: extension stuck in a startup for users without email
2 participants