Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Mouser Order Import only included first 30 items out of 48 #244

Closed
dubajj opened this issue Jan 3, 2024 · 8 comments · Fixed by #255
Closed

[Bug]: Mouser Order Import only included first 30 items out of 48 #244

dubajj opened this issue Jan 3, 2024 · 8 comments · Fixed by #255
Assignees
Labels
bug Something isn't working
Milestone

Comments

@dubajj
Copy link

dubajj commented Jan 3, 2024

Binner version

v2.6.0

Operating System

Windows

Describe the bug and the steps to reproduce it

I imported my most recent order from mouser which has 48 line items. Only the first 30 show up in the order import view

Would you like to attach your appsetings.json configuration?

No response

Screenshots or Videos (Optional, but they help!)

No response

Are you able to contribute a PR? (No is ok!)

None

@dubajj dubajj added the bug Something isn't working label Jan 3, 2024
@dubajj
Copy link
Author

dubajj commented Jan 4, 2024

built from source this evening and in public async Task<IApiResponse> GetOrderAsync(string orderId, Dictionary<string, string>? additionalOptions = null) i can see that all 48 items are returned from mouser, and parsed correctly and passed to return new ApiResponse(results, nameof(MouserApi));

I guess that means its a problem with the front end or dB? Something else is surely going wrong as the response from mousers servers takes about 1/3 a second but the page hangs (in production and debug) for a full minute after that response before it shows anything at all, and then it only shows 30 out of the 48 items

@replaysMike
Copy link
Owner

Hi @dubajj - I'll look today to see if there's an issue. Sounds to me like a frontend issue, but we haven't pushed any releases in a while as I've been away. I'll take a look at your other tickets as well.

@replaysMike replaysMike self-assigned this Jan 8, 2024
@dubajj
Copy link
Author

dubajj commented Jan 8, 2024

i joined the discord let me know if i can help out.

@shaun-leach
Copy link
Contributor

Ran into this issue myself. Looks like the problem is that Mouser will occasionally return an error code of Forbidden when calling GetProductDetailsAsync here:

https://github.com/replaysMike/Binner/blob/bed3ea6d7519d5adea05c08f2b848351f348ac46/Binner/Library/Binner.Common/Services/PartService.cs#L356C41-L356C41

@shaun-leach
Copy link
Contributor

It looks like if you retry once or twice it will return without an error code.

@replaysMike replaysMike added this to the v2.6.1 milestone Mar 12, 2024
@replaysMike replaysMike added the in-progress This ticket is currently being worked on label Mar 12, 2024
@replaysMike
Copy link
Owner

thanks for the tips, I'm looking into this now and hopefully I can repro.

@replaysMike
Copy link
Owner

ok I think I understand what the issue is, haven't looked at this code in a long time. Initially I thought Mouser's api may be paginating the data, but there are no such options in their api.

The Mouser api for grabbing order history details doesn't return much data as part of the response, so Binner requests more information from their product info api for each line item in the order. For larger orders they may be throttling the requests after so many are made, and Binner isn't handling that properly (because it's difficult to trigger during testing). This is also likely slow depending on how long each product request takes from their api.

I've asked for more information from Mouser on this behavior, meanwhile I'll try to harden the code to handle this better and refamiliarize myself with the orders api again.

@replaysMike
Copy link
Owner

This is now fixed and will appear in the v2.6.1 release.

@replaysMike replaysMike removed the in-progress This ticket is currently being worked on label Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants