cosmicjs / cosmic-sdk-js

The official JavaScript SDK for Cosmic. Use it to power content server-side, in the browser, and in native apps.
https://www.npmjs.com/package/@cosmicjs/sdk
MIT License
14 stars 2 forks source link

[FIX] Promises Thenable Type, Media Find Type #19

Closed rajatkaush1k closed 1 year ago

rajatkaush1k commented 1 year ago

Also includes eslint updates & a test file with my dummy configuration. Contains the fix for Issues/18

rajatkaush1k commented 1 year ago

Waiting for @jazibsawar's approval. Will remove the test file and merge once approved.

jazibsawar commented 1 year ago

I refactored to clean up the code. One scenario is not working though,

cosmic.media.findOne({})
  .then(res => console.log(res))
  .catch(err => console.log('THIS NEVER GETS CALLED EVEN ERROR');
rajatkaush1k commented 1 year ago

I refactored to clean up the code. One scenario is not working though,

cosmic.media.findOne({})
  .then(res => console.log(res))
  .catch(err => console.log('THIS NEVER GETS CALLED EVEN ERROR');

Thanks for the cleanup. I've included a new condition in findOne method for the parameter object to not be empty. Looks like the condition never goes into the catch block because the request pulls through successfully in the try block, which it shouldn't - making me wonder if we're considering this as a valid request on our backend.

jazibsawar commented 1 year ago

I refactored to clean up the code. One scenario is not working though,

cosmic.media.findOne({})
  .then(res => console.log(res))
  .catch(err => console.log('THIS NEVER GETS CALLED EVEN ERROR');

Thanks for the cleanup. I've included a new condition in findOne method for the parameter object to not be empty. Looks like the condition never goes into the catch block because the request pulls through successfully in the try block, which it shouldn't - making me wonder if we're considering this as a valid request on our backend.

It wasn't about an error from the backend. It doesn't go to catch even if there is an error. I tested it by throwing an error

rajatkaush1k commented 1 year ago

I refactored to clean up the code. One scenario is not working though,

cosmic.media.findOne({})
  .then(res => console.log(res))
  .catch(err => console.log('THIS NEVER GETS CALLED EVEN ERROR');

Thanks for the cleanup. I've included a new condition in findOne method for the parameter object to not be empty. Looks like the condition never goes into the catch block because the request pulls through successfully in the try block, which it shouldn't - making me wonder if we're considering this as a valid request on our backend.

It wasn't about an error from the backend. It doesn't go to catch even if there is an error. I tested it by throwing an error

I tried passing in an invalid string value (for which it should throw an error) and it did the error handling correctly for me.

Can you try with .findOne({ original_name: 'bad_request' }) in the test file to confirm?

In your specific case with the empty object ({}) it did not go into the catch statement.

jazibsawar commented 1 year ago

I refactored to clean up the code. One scenario is not working though,

cosmic.media.findOne({})
  .then(res => console.log(res))
  .catch(err => console.log('THIS NEVER GETS CALLED EVEN ERROR');

Thanks for the cleanup. I've included a new condition in findOne method for the parameter object to not be empty. Looks like the condition never goes into the catch block because the request pulls through successfully in the try block, which it shouldn't - making me wonder if we're considering this as a valid request on our backend.

It wasn't about an error from the backend. It doesn't go to catch even if there is an error. I tested it by throwing an error

I tried passing in an invalid string value (for which it should throw an error) and it did the error handling correctly for me.

Can you try with .findOne({ original_name: 'bad_request' }) in the test file to confirm?

In your specific case with the empty object ({}) it did not go into the catch statement.

Are you using try-catch? Try catch do work. But then().catch() doesn't for me. I'll try again

rajatkaush1k commented 1 year ago

I refactored to clean up the code. One scenario is not working though,

cosmic.media.findOne({})
  .then(res => console.log(res))
  .catch(err => console.log('THIS NEVER GETS CALLED EVEN ERROR');

Thanks for the cleanup. I've included a new condition in findOne method for the parameter object to not be empty. Looks like the condition never goes into the catch block because the request pulls through successfully in the try block, which it shouldn't - making me wonder if we're considering this as a valid request on our backend.

It wasn't about an error from the backend. It doesn't go to catch even if there is an error. I tested it by throwing an error

I tried passing in an invalid string value (for which it should throw an error) and it did the error handling correctly for me. Can you try with .findOne({ original_name: 'bad_request' }) in the test file to confirm? In your specific case with the empty object ({}) it did not go into the catch statement.

Are you using try-catch? Try catch do work. But then().catch() doesn't for me. I'll try again

Ah, gotcha. I was using Try-catch in the function. Just tried with the chaining methods and catch isn't working. Will look into whether this issue got introduced with this PR or is old.