tweedegolf / storage-abstraction

Provides an abstraction layer for interacting with a storage; the storage can be local or in the cloud.
MIT License
106 stars 18 forks source link

Accessing promise index instead of index' result #67

Closed CaporalDead closed 3 months ago

CaporalDead commented 3 months ago

Summary:

There is a priority problem between the promise/await and array access. Fix this by prioritizing the promise result over then array access. It was causing the following error :

/home/XXXXX/project/src/applications/business/XXXX.ts:5
import {useStorage} from "../../storage";
                                                         ^
Error: An error occurred while trying to generate a signed URL
    at /home/XXXXX/project/src/applications/business/XXXX.ts:114:15
    at Generator.next (<anonymous>)
    at fulfilled (/home/XXXXX/project/src/applications/business/XXXX.ts:5:58)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)

The code to reproduce

const storage = new Storage({
  type: StorageType.GCS,
  keyFilename: '/path/key.json',
  bucketName: 'my-bucket',
});

await storage.getFileAsURL(data.file.path, {
  useSignedUrl: true,
  expiresOn: Date.now() + 1000 * 60 * 60, // one hour
});

Tested on:

Node 20.12.2 NPM 10.5.0

Fix proposal

See https://github.com/tweedegolf/storage-abstraction/pull/66

CaporalDead commented 3 months ago

Hey guys, any chance to see this fix reviewed (and merged :grin:) ?

Thanks for your feedback :pray: !

CaporalDead commented 3 months ago

Thanks @abudaan :pray: . I didn't update the version number but I wrote the CHANGELOG with the Unreleased tag. Didn't know if I had to bump the version number manually.

CaporalDead commented 3 months ago

Oupsy, I just saw that I think that I didn't update the right CHANGELOG oO. Do I have to make a new PR to fix this ?

abudaan commented 3 months ago

No problem :)

Thanks for your fix!

CaporalDead commented 3 months ago

@abudaan , I think that the npm update for the GCS adapter is missing, it's still 1.0.7 instead of 1.0.8 :) (https://www.npmjs.com/package/@tweedegolf/sab-adapter-google-cloud)

CaporalDead commented 3 months ago

Hi @abudaan, me again :sweat_smile: , sorry to bother you (again) but I think something went wrong with the build. I saw you tagged 1.0.8 but the output code does not include the patch.

I git cloned your last commit, run npm i && npm run prepare to replay the build as I did before and I can see that the fix is present in the publish/AdapterGoogleCloud/dist/AdapterGoogleCloud.js file.

Not sure of what's going on here :thinking: