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

[PM-5177] fix CLI list items lock error #7133

Merged
merged 6 commits into from
Dec 15, 2023
Merged

[PM-5177] fix CLI list items lock error #7133

merged 6 commits into from
Dec 15, 2023

Conversation

bxb100
Copy link
Contributor

@bxb100 bxb100 commented Dec 7, 2023

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

fix #7126

Code changes

apps/cli/src/commands/list.command.ts: sync run this.eventCollectionService.collect

the execution stack is:

const cipher = await this.cipherService.get(cipherId);

async get(id: string): Promise<Cipher> {
const ciphers = await this.stateService.getEncryptedCiphers();

await this.getAccount(this.reconcileOptions(options, await this.defaultOnDiskMemoryOptions()))

: await this.storageService.get<TAccount>(options.userId, options);

obviously proper-lockfile lock showed the error is typically lock race

Screenshots

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team
  • Ensure that all UI additions follow WCAG AA requirements

@bitwarden-bot
Copy link

Thank you for your contribution! We've added this to our internal Community PR board for review.
ID: PM-5177

@CLAassistant
Copy link

CLAassistant commented Dec 7, 2023

CLA assistant check
All committers have signed the CLA.

@bitwarden-bot bitwarden-bot changed the title fix CLI list items lock error [PM-5177] fix CLI list items lock error Dec 7, 2023
@bitwarden-bot
Copy link

bitwarden-bot commented Dec 7, 2023

Logo
Checkmarx One – Scan Summary & Detailsc8a3cac0-7ec9-4c24-a04a-bf0b5dab9283

No New Or Fixed Issues Found

@djsmith85 djsmith85 mentioned this pull request Dec 13, 2023
1 task
@ttalty ttalty self-assigned this Dec 13, 2023
@ttalty ttalty added the needs-qa Marks a PR as requiring QA approval label Dec 13, 2023
Copy link
Contributor

@ttalty ttalty left a comment

Choose a reason for hiding this comment

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

This fixes the issue on the lock file being held.

@ttalty
Copy link
Contributor

ttalty commented Dec 13, 2023

Thank you for the contribution! This PR has been approved and is moving to QA for testing.

@ttalty ttalty removed the needs-qa Marks a PR as requiring QA approval label Dec 15, 2023
@ttalty ttalty merged commit 6199e58 into bitwarden:main Dec 15, 2023
11 of 13 checks passed
@Geezus42
Copy link

When will the release be cut? I've already delt with a broken work flow for a week, is it going to be two? It's not a good sign that this was not caught by your QA.

ttalty pushed a commit that referenced this pull request Dec 19, 2023
Co-authored-by: Tom <144813356+ttalty@users.noreply.github.com>
(cherry picked from commit 6199e58)
@jabofh
Copy link

jabofh commented Feb 8, 2024

Since I'm still struggling with this issue, have you any idea when it'll be generally available?

macosver
10.16
❯
❯ ./bw --version
2024.2.0
❯ 
❯ time ./bw list items --session "$( cat ~/.bw_session )" > /dev/null
./bw list items --session "$( cat ~/.bw_session )" > /dev/null  75.39s user 18.67s system 151% cpu 1:02.10 total

Still not the less-than-30-seconds it takes on my Fedora box...

eroux@fedoravm:~$ time bw list items --session "$( cat ~/.bw_session )" > /dev/null

real	0m23.664s
user	0m19.311s
sys	0m5.879s
eroux@fedoravm:~$ 

@bxb100
Copy link
Contributor Author

bxb100 commented Feb 8, 2024

@jabofh you may need to check lowdb/adapters/FileSync how to prolong the execution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: CLI list item error
6 participants