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

Docs: Fix possible NullReferenceException and Cross-Origin Request Blocked #6647

Merged
merged 4 commits into from
Apr 13, 2023

Conversation

ScarletKuro
Copy link
Member

Description

I think this should fix the problem reported by @Mr-Technician

How Has This Been Tested?

Visual

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added bug Something does not work as intended/expected PR: needs review labels Apr 13, 2023
@ScarletKuro ScarletKuro requested a review from henon April 13, 2023 11:25
@Mr-Technician
Copy link
Member

I'm still confused as to how/why this is showing up only on iOS mobile.

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Apr 13, 2023

I'm still confused as to how/why this is showing up only on iOS mobile.

I believe that the issue you encountered might be either a coincidence or related to some problem with HttpClient on iOS. From the stack trace you provided, the exception occurred at BuilderRenderTree>b__0_135.
By de-compiling and examining this method, we can see that the problem is likely caused by the _nugetPackage object, as the following line indicates: __builder5.AddContent(745, ((Decimal) this._nugetPackage.TotalDownloads).RoundedToString());.
that the only line of code in the BuilderRenderTree>b__0_135.

@ScarletKuro
Copy link
Member Author

related to some problem with HttpClient on iOS.

It would be nice to see the console log since it's wrapped with try-catch, but I think it's not possible on iOS.

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Apr 13, 2023

@Mr-Technician I just got the same error on PC with Firefox which means its not iOS related, it looks like just a common problem when API is down or we get rate limited.

@Mr-Technician
Copy link
Member

Mr-Technician commented Apr 13, 2023

related to some problem with HttpClient on iOS.

It would be nice to see the console log since it's wrapped with try-catch, but I think it's not possible on iOS.

For future reference, this should be possible using inspect://chrome, which logs on iOS chrome console messages into a separate tab.

@Mr-Technician I just got the same error on PC with Firefox which means its not iOS related, it looks like just a common problem when API is down or we get rate limited.

This would make sense

@ScarletKuro
Copy link
Member Author

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://azuresearch-usnc.nuget.org/query?q=packageid:mudblazor&take=1. (Reason: CORS request did not succeed). Status code: (null).
On some browsers we get this.

@ScarletKuro
Copy link
Member Author

Doesn't seems that setting cors on WasmHost has any effect :(

@ScarletKuro
Copy link
Member Author

Ok I found a way to fix the cross origin, but I will update the PR later. I need to polish the code and I'm busy rn.

@henon
Copy link
Collaborator

henon commented Apr 13, 2023

LGTM, is it ready for merge?

@ScarletKuro
Copy link
Member Author

ScarletKuro commented Apr 13, 2023

I have added a fix for the Cross-Origin Request Blocked issue. Ideally, we should have used a typed/named HttpClient, but it would have required adding the additional Microsoft.Extensions.Http library to MudBlazor.Docs and changing all the samples that use HttpClient directly to use IHttpClientFactory or a typed wrapper for the backend calls. Instead, I have taken a different approach by creating own instance of HttpClient in DiscordApiClient, NugetApiClient, and GitHubApiClient.

Changed the lifetime of these Clients instances from scoped to singleton to prevent injection of an HttpClient that is related to localhost. This ensures that the HttpClient instances act as singletons across the entire application and will be correctly dispose HttpClient during application shutdown. I use a similar approach at work in some of the WebAssembly projects that have multiple API clients.

@ScarletKuro
Copy link
Member Author

LGTM, is it ready for merge?

Now yes.

@ScarletKuro ScarletKuro changed the title Docs: Fix possible NullReferenceException Docs: Fix possible NullReferenceException and Cross-Origin Request Blocked Apr 13, 2023
@henon henon merged commit ba386dd into MudBlazor:dev Apr 13, 2023
3 checks passed
@henon
Copy link
Collaborator

henon commented Apr 13, 2023

👍

ScarletKuro added a commit that referenced this pull request Aug 19, 2023
…ocked (#6647)

* Docs: Fix possible NullReferenceException
* Fix Cross-Origin Request Blocked
ilovepilav pushed a commit to ilovepilav/MudBlazor that referenced this pull request Nov 25, 2023
…ocked (MudBlazor#6647)

* Docs: Fix possible NullReferenceException
* Fix Cross-Origin Request Blocked
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as intended/expected PR: needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants