MatrixAI / Polykey

Polykey Core Library
https://polykey.com
GNU General Public License v3.0
31 stars 4 forks source link

`VaultsSecrets` handlers don't return a proper error object #826

Closed aryanjassal closed 5 days ago

aryanjassal commented 1 month ago

Specification

Similarly, both handlers return a string if an error happens. This is not ideal. Instead, an error object like SuccessOrErrorMessage can instead be returned, and the error string can be built by the client themselves. This is done to an extent by VaultsSecretsMkdir (see Additional context), but that approach is still incorrect, as VaultOps returns an error object instead of throwing an error. All VaultOps operations should throw an error, which should be handled and converted as necessary by the handler itself.

Additional context

// VaultsSecretsMkdir
yield await vaultManager.withVaults(
  [vaultId],
  async (vault) => {
    return await vaultOps.mkdir(vault, dirName, {
      recursive: metadata?.options?.recursive,
    });
  },
  tran,
);

// vaultOps.mkdir()
try {
  await vault.writeF(async (efs) => {
    await efs.mkdir(dirPath, fileOptions);
    logger?.info(`Created secret directory at '${dirPath}'`);
  });
  return { type: 'success', success: true };
} catch (e) {
  logger?.error(`Failed to create directory '${dirPath}'. Reason: ${e.code}`);
  if (e.code === 'ENOENT' && !recursive) {
    return {
      type: 'error',
      code: e.code,
      reason: dirPath,
    };
  }
  if (e.code === 'EEXIST') {
    return {
      type: 'error',
      code: e.code,
      reason: dirPath,
    };
  }
  throw e;
}

Tasks

  1. Return a proper SuccessOrErrorMessage from VaultsSecretsRemove and VaultsSecretsGet
  2. Update VaultOps.mkdir to throw errors rather than returning an error object
  3. Go through all the tests for them all and ensure everything is up to date
  4. Use fastcheck where possible to test these things out
linear[bot] commented 1 month ago

ENG-428 `VaultsSecretsRemove` does not fully align with UNIX expectations

linear[bot] commented 3 weeks ago

ENG-428 `VaultsSecrets` handlers don't return a proper error object

aryanjassal commented 5 days ago

This issue was actually resolved by Polykey#838, but I forgot to mark it as related. Closing.