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]: Refit 8 breaks ability to cast IApiResponse<T> to IApiResponse #1949

Closed
snake-scaly opened this issue Jan 20, 2025 · 5 comments
Closed
Labels

Comments

@snake-scaly
Copy link

snake-scaly commented Jan 20, 2025

Describe the bug 🐞

The PR #1879 breaks our old code. Our API uses the generic version of IApiResponse<T>, but there are some implementation methods which accept the base IApiResponse. This works fine in 7.2.22. But in 8.0.0 IApiResponse<T>.Error and IApiResponse.Error are different fields, so in this case IApiResponse.Error is always null no matter what.

Steps to reproduce

In pseudocode:

interface Api
{
  [Get("/resource")]
  Task<IApiResponse<string>> Foo();
}

async Task Bar()
{
  var client = RestService.For<Api>("https://example.com");
  var response = await client.Foo();
  Verify(response);
}

void Verify(IApiResponse r)
{
  if (r.Error != null) throw r.Error; // never happens
}

Expected behavior

Applications should be able to use the base interface to access common response fields.

Refit Version

8.0.0

@snake-scaly snake-scaly changed the title [Bug]: [Bug]: Refit 8 breaks ability to cast IApiResponse<T> to IApiResponse Jan 21, 2025
@sguryev
Copy link
Contributor

sguryev commented Jan 22, 2025

Duplicate of #1933

@sguryev
Copy link
Contributor

sguryev commented Jan 22, 2025

Not sure if I got your sample but I can't reproduce it here

Could you please provide the PR with test?

@snake-scaly
Copy link
Author

So my bug report is incorrect. The production code will still work, it's just the mocking that's the problem. Good to know. Still, introducing a breaking change (even if just in tests) for slightly nicer IDE reporting seems excessive.

@sguryev
Copy link
Contributor

sguryev commented Jan 22, 2025

Yep. It's a "kind of" breaking change. We need to find a tradeoff between mocking correct interfaces and using NRT annotations. And it's not about IDE reporting. It's analyzers (with warning as errors msbuild reports build errors)

@snake-scaly
Copy link
Author

Closing as a duplicate, because #1933 is a much better description of the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants