Sm/aliases-without-config-file #842
Conversation
| try { | ||
| await unlock(this.fileLocation); | ||
| } catch { | ||
| // ignore the error. If it wasn't locked, that's what we wanted |
There was a problem hiding this comment.
Is that the only error that can happen when unlocking?
There was a problem hiding this comment.
good point. I'll make the error specific to the reason I was doing this. There could be others that are fs related.
| await this.saveAliasStoreToFile(); | ||
| return; | ||
| } | ||
| if (useLock) await unlock(this.fileLocation); |
There was a problem hiding this comment.
This is an edge case, but seems possible:
lock throws an error that is not an ENOENT (maybe an EPERM error) so the lock is never obtained. In the catch we try to unlock and that throws an error, and is the one reported to the user. The user wouldn't know why the lock couldn't be obtained. To prevent this, we might need to use a try/catch/finally and the finally has a try/catch for the unlock.
There was a problem hiding this comment.
I think putting the original lock outside the try should surface an error better.
|
QA Notes:
|
What does this PR do?
stateAggregator.aliasAccessor without configFile.
every set/unset creates an fs operation with file locking (new library dependency, 3PP)
deprecate things that only AliasAccessor was using (aliasConfig, configGroup) to get to configFile
testSetup has special "mocks" for aliases now that they're not using configFile (they're actually real fs ops to the tmp dir)
this would be a breaking change to the public API because the inheritane chain from aliasAccessor contained a LOT of methods we aren't using and now it won't.
What issues does this PR fix or reference?
@W-12065678@ forcedotcom/cli#1806