duffelhq / duffel-api-javascript

JavaScript client library for the Duffel API
https://duffel.com/docs
MIT License
35 stars 12 forks source link

Undefined booking response, no errors returned #704

Closed godoyLeonardo closed 1 year ago

godoyLeonardo commented 1 year ago

Hello!

We are in the services part regarding the JS SDK integration but we are running into a problem. When trying to Create an Order, the response is undefined and the SDK does not return errors.

It had happened before but it turned out to be because of a sum error in the "amount" (which also doesn't return errors when it's wrong as far as we've tested). But now, with several services, for some reason it fails and we believe that this time the sum is correct. The same booking, but with less baggage services, did work.

We also enabled debug mode in the SDK to check more accurately but it didn't help much.

The log and the information with which we tried to create an order, in the test environment:

 [2023-03-23T02:26:25.815Z] Endpoint:  https://api.duffel.com/air/orders
 [2023-03-23T02:26:25.815Z] Method:  POST
 [2023-03-23T02:26:25.815Z] Body Parameters:  {
   selected_offers: [ 'off_0000ATtQPgBS0joLrbNcY7' ],
   payments: [ { type: 'balance', currency: 'EUR', amount: '253.91' } ],
   passengers: [
     {
       type: 'adult',
       loyalty_programme_accounts: [],
       id: 'pas_0000ATtQPdvQPLnsrJrnHu',
       given_name: 'Leonardo',
       family_name: 'Godoy',
       age: null,
       born_on: '1980-02-02',
       email: 'REDACTED',
       phone_number: 'REDACTED',
       gender: 'm',
       title: 'mr'
     }
   ],
   type: 'instant',
   services: [
     { id: 'ase_0000ATtQPlOz1hvW9V553C', quantity: 1 },
     { id: 'ase_0000ATtQPlOz1hvW9V553A', quantity: 3 },
     { id: 'ase_0000ATtQPlOz1hvW9V553B', quantity: 3 }
   ]
 }
go1t commented 1 year ago

hi @godoyLeonardo, thanks for reaching out, and yes the error should have been there. To clarify, the error won't be returned by the call but rather will be thrown in a form of DuffelError class instead. Have you tried surrounding the call with try-catch?

As for the error here in particular, I checked the log and the request failed due to the error in the body parameter (the response code was supposed to be 422) - the error in particular is Field 'services' expected one baggage service per bag type, passenger, and segment. In this particular case, it's likely due to two types of checked bag service being added simultaneously. Let me know if that helps!

godoyLeonardo commented 1 year ago

Hi @go1t ! thanks for your help.

hi @godoyLeonardo, thanks for reaching out, and yes the error should have been there. To clarify, the error won't be returned by the call but rather will be thrown in a form of DuffelError class instead. Have you tried surrounding the call with try-catch?

yes, it is wrapped in a try catch, but the Duffel Error is not there. The response is undefined.

As for the error here in particular, I checked the log and the request failed due to the error in the body parameter (the response code was supposed to be 422) - the error in particular is Field 'services' expected one baggage service per bag type, passenger, and segment. In this particular case, it's likely due to two types of checked bag service being added simultaneously. Let me know if that helps!

Could you expand on this limitation? Is there a Max_baggage or max_services we should look into besides the quantity in the Offer's services?

If I recall correctly, maybe it was a different booking, it also failed when we attempted to book one hand baggage, 3 15kg hold and 3 23kg hold, same options found in the order management menu in app.duffel

Lastly, is there a method or endpoint we can call to read this errors when they happen?

go1t commented 1 year ago

@godoyLeonardo No problem!

yes, it is wrapped in a try catch, but the Duffel Error is not there. The response is undefined.

I see. This might be an error on our side as the expected behavior here is that the SDK should throw a DuffelError. I will look into it tomorrow and will get back to you, if that's alright.

Could you expand on this limitation? If I recall correctly, maybe it was a different booking, it also failed when we attempted to book one hand baggage, 3 15kg hold and 3 23kg hold

As far as I understand, when there are multiple services with the same baggage type (e.g. the 15kg and 23kg checked back you mentioned), we don't allow selecting both at the moment. So you may have to display the UI where the user has to choose between them before adding the quantity.

same options found in the order management menu in app.duffel

I also just found out about this restriction recently, so our dashboard hasn't been updated to display that properly. We will look into updating that along with the baggage component soon. Sorry for the confusion!

godoyLeonardo commented 1 year ago

@go1t Now everything is a little clearer, thanks!

I checked again and there is a try catch. Also, as I said, this situation of not having errors happens (as far as I know) when the total_price is incorrect or when there are problems with the services.

  try {
    bookingResponse = await duffel.orders.create(duffelBooking);
  } catch (error) {
    logError(error, 'Duffel booking error')
    return undefined;
  }

  const something = doSomething(bookingResponse.data) // It fails here because data does not exist and the code did not stop at the try catch.

and to summarize, and to confirm that I understood correctly the baggage problem.

When we are presented with services such as:

[
    {type: hand, weight: 13kg max_quantity: 2}
    {type: hold, weight: 20kg, max_quantity: 3}
    {type: hold, weight: 25kg, max_quantity: 4}
]

The user must be presented with the options:

Hand => up to 2
&&
(
    Hold 20kg => up to 3
    ||
    Hold 25kg => up to 4
)

With the price being calculated by service.price multiplied by the number of times that service was selected.

go1t commented 1 year ago

@godoyLeonardo That's correct 👍

go1t commented 1 year ago

@godoyLeonardo Thank you! That is very strange indeed.

I have just tried using the SDK myself again and it is able to throw properly. Could you please also confirm your SDK version and node version?

godoyLeonardo commented 1 year ago

@go1t

Node version v16.5.1 Duffel version 1.25.0

go1t commented 1 year ago

@godoyLeonardo Thanks. Unfortunately, I don't have an answer on why this happened at the moment since I haven't managed to reproduce this at all- I will need to look into this more closely tomorrow. Meanwhile, perhaps you could try removing node_modules and installing the SDK again and see if the issue goes away or not. And/or maybe try triggering an error on different endpoints (e.g. searching flight with a past date) and see if it shows up or not (just to help narrow down whether it's an SDK-wide problem or the endpoint problem).

You could also check out the code on how the SDK makes API request here

godoyLeonardo commented 1 year ago

@go1t

I'll do those recommendations today, thank you very much.

On a similar topic and while we fix this issue, could you provide me with a little more info about this problem?

I'll leave you the log:

[2023-03-23T19:29:26.909Z] Endpoint:  https://api.duffel.com/air/orders
[2023-03-23T19:29:26.910Z] Method:  POST
[2023-03-23T19:29:26.910Z] Body Parameters:  {
  selected_offers: [ 'off_0000ATuuMBp6IeEwfiEYig' ],
  payments: [ { type: 'balance', currency: 'EUR', amount: '167.62' } ],
  passengers: [
    {
      type: 'adult',
      loyalty_programme_accounts: [],
      id: 'pas_0000ATuuM90gl6j7wmdGra',
      given_name: 'ASDASD',
      family_name: 'asad',
      age: null,
      born_on: '1980-02-02',
      email: 'asd@asd.com',
      phone_number: 'REDACTED',
      gender: 'm',
      title: 'mr'
    }
  ],
  type: 'instant',
  services: [
    { id: 'ase_0000ATuuMI79TJS4Og7aT4', quantity: 1 },
    { id: 'ase_0000ATuuMI79TJS4Og7aT3', quantity: 3 }
  ]
}

Services from that offer that we displayed to the user:

[
  {
    "type": "baggage",
    "total_currency": "EUR",
    "total_amount": "31.12",
    "segment_ids": ["seg_0000ATuuMBp6IeEwfiEYie"],
    "passenger_ids": ["pas_0000ATuuM90gl6j7wmdGra"],
    "metadata": {
      "type": "checked",
      "maximum_weight_kg": 23,
      "maximum_length_cm": null,
      "maximum_height_cm": null,
      "maximum_depth_cm": null
    },
    "maximum_quantity": 3,
    "id": "ase_0000ATuuMI79TJS4Og7aT2"
  },
  {
    "type": "baggage",
    "total_currency": "EUR",
    "total_amount": "26.03",
    "segment_ids": ["seg_0000ATuuMBp6IeEwfiEYie"],
    "passenger_ids": ["pas_0000ATuuM90gl6j7wmdGra"],
    "metadata": {
      "type": "checked",
      "maximum_weight_kg": 15,
      "maximum_length_cm": null,
      "maximum_height_cm": null,
      "maximum_depth_cm": null
    },
    "maximum_quantity": 3,
    "id": "ase_0000ATuuMI79TJS4Og7aT3"
  },
  {
    "type": "baggage",
    "total_currency": "EUR",
    "total_amount": "18.10",
    "segment_ids": ["seg_0000ATuuMBp6IeEwfiEYie"],
    "passenger_ids": ["pas_0000ATuuM90gl6j7wmdGra"],
    "metadata": {
      "type": "carry_on",
      "maximum_weight_kg": null,
      "maximum_length_cm": null,
      "maximum_height_cm": null,
      "maximum_depth_cm": null
    },
    "maximum_quantity": 1,
    "id": "ase_0000ATuuMI79TJS4Og7aT4"
  }
]
go1t commented 1 year ago

@godoyLeonardo The error in the above case is actually insufficient balance in the wallet. However, to my understanding this shouldn't happen as on the test mode we credit your balance automatically whenever a booking happens. I will check with the team tomorrow to see what might be going on here.

godoyLeonardo commented 1 year ago

@go1t

I already re-installed the @Duffel/Api but as you can infer at this point, that error didn't show up for me either hahaha.

I'll keep trying other ways to see if I can fix it. Thank you very much!

Edit: Different token (using a different Duffel account we have). Same problem, no error, booking is undefined.

[2023-03-23T20:32:44.688Z] Endpoint:  https://api.duffel.com/air/orders
[2023-03-23T20:32:44.688Z] Method:  POST
[2023-03-23T20:32:44.688Z] Body Parameters:  {
  selected_offers: [ 'off_0000ATuzpDDIi1ih9WDB6e' ],
  payments: [ { type: 'balance', currency: 'EUR', amount: '332.90' } ],
  passengers: [
    {
      type: 'adult',
      loyalty_programme_accounts: [],
      id: 'pas_0000ATuzp1H761di7ru3Nq',
      given_name: 'ASDASD',
      family_name: 'asad',
      age: null,
      born_on: '1980-02-02',
      email: 'asd@asd.com',
      phone_number: 'REDACTED',
      gender: 'm',
      title: 'mr'
    },
    {
      type: 'adult',
      loyalty_programme_accounts: [],
      id: 'pas_0000ATuzp1H761di7ru3Nr',
      given_name: 'qweqweqew',
      family_name: 'weqweqew',
      age: null,
      born_on: '1977-02-02',
      email: 'asdads@asd.com',
      phone_number: 'REDACTED',
      gender: 'm',
      title: 'mr'
    }
  ],
  type: 'instant',
  services: [
    { id: 'ase_0000ATuzpZzR1oGeiBIfeC', quantity: 1 },
    { id: 'ase_0000ATuzpZzR1oGeiBIfeD', quantity: 1 }
  ]
}
go1t commented 1 year ago

@godoyLeonardo :( I'd recommend to perhaps try to recreate a small example for it just to see if you can get the error logged or not. You can use this gist as well https://gist.github.com/go1t/b2cd1b23c37f3cd784bf3720d0591a86 (I have tested this on node 16.19.1 and lts-hydrogen with sdk version 1.24 and the error was caught properly).

godoyLeonardo commented 1 year ago

@go1t

Could this help a little more? I made this seach fail using yesterday as date. I also updated node to v16.19.1.

As package manager we are using Yarn and we use Docker, I can't think about any other significant or important difference that could be causing this weird behaviour.

[debug: 23/03 20:45:26] Duffel search request for arrival SCL and destination CCP with passengers [{"age":30},{"age":30}]
[2023-03-23T20:45:26.433Z] Endpoint:  https://api.duffel.com/air/offer_requests/?return_offers=true
[2023-03-23T20:45:26.433Z] Method:  POST
[2023-03-23T20:45:26.433Z] Body Parameters:  {
  slices: [
    {
      origin: 'SCL',
      destination: 'CCP',
      departure_date: '2023-03-22T20:45:26.431Z',
      destination_type: 'airport',
      origin_type: 'airport'
    }
  ],
  passengers: [
    { family_name: undefined, given_name: undefined, age: 30 },
    { family_name: undefined, given_name: undefined, age: 30 }
  ],
  cabin_class: 'economy'
}
[2023-03-23T20:45:26.434Z] Query Parameters:  { return_offers: true }
[error: 23/03 20:45:26] at O.next (/usr/app/node_modules/@duffel/api/node_modules/tslib/tslib.es6.js:193:1) at Generator.next (<anonymous>) at a (/usr/app/node_modules/@duffel/api/node_modules/tslib/tslib.es6.js:73:58)
Error // This is a console.log('Error', error);
go1t commented 1 year ago

@godoyLeonardo Thanks. Looking at the logs above, it does seem like the error is being logged from within tslib, so that's good to see I think? I wonder if this has something to do with the transpiilation target. Taking a shot in the dark here, would you be able to check your transpilation target and whether node_modules is being excluded? Maybe try calling it without await (e.g. using .catch()) and see if that works out.

Error // This is a console.log('Error', error);

Are you saying that the catch works in this case and it's just the error that was being empty?

If all else fails, there is also an option to call the API directly (we have an API reference on https://duffel.com/docs). Obviously that would require slightly more set up, but at least you would still be able to import the types from the SDK.

go1t commented 1 year ago

or if you could create a small repo that shows how this doesn't work for you, that would allow us to help debugging this more easily.

godoyLeonardo commented 1 year ago

@go1t

Hi, thanks for all your help.

After a bit of debugging we realized that the error is there, yes, but for some reason that may be our fault, console.log() ,logger and logError did not show the actual error. console.dir()did the trick.

Thanks to this we identified a "new" error in the booking process, this time it is "Field 'currency' didn't match GBP", requestId: F09hYFIxMqxvVqEAJqMB.

I think this is because yesterday, to see if we could bypass the lack of funds problem you mentioned, I started using the token from the account you originally created for us. Do we need to configure something to get the prices in euros?

go1t commented 1 year ago

@godoyLeonardo Nice, I'm glad you got to the bottom of it 🎉

Thanks to this we identified a "new" error in the booking process, this time it is "Field 'currency' didn't match GBP", requestId: F09hYFIxMqxvVqEAJqMB.

Generally, the currency field here should match the currency of the organisation (it's an extra safeguard we have to ensure that you work with the right currency). So if you are looking to get the prices / booking in euros, you would have to use the token from euro organisation.

As for the lack of funds problem, I have passed this along to our team and we identified the issue to occur when multiple quantity of services are added. I will keep you updated when that is fixed. For now, if you only have at most one quantity selected for each service, the order should still go through. So hopefully that should allow you to continue using your EUR org.

I believe we also have set up a channel between Duffel and your team, so if you prefer, we can continue the conversations and supports there (as this might be beyond the scope of the github issue at this point 😅 )

godoyLeonardo commented 1 year ago

Thanks!

Please send me a message through slack so I can know your @handle there.