Failing at API dev

Solarion said:
Any thoughts or feedback welcome!!
Click to expand...
If it works lol but the chances of you/someone changing an API to cater for a client/you is very rare.
It’s probably throwing an exception because you’re not checking if the response.Content is null?
 
DA-LION-619 said:
If it works lol but the chances of you/someone changing an API to cater for a client/you is very rare.
It’s probably throwing an exception because you’re not checking if the response.Content is null?
Click to expand...

Yeah no you are right, a 404 is the way to go for a non existent record. Basically does mean the user screwed up lol.

The other option say for an empty array is to return an Ok(new List<T>());

If searching for say a User with a particular Id and it doesn't exist then a 404 may be better because, why would anyone ever do that unless they fluffed something on the UI side.

Just some thought on that, thinking out loud here.

I will check that response content and link the relevant code which shows that a 404 on my project registers as an error in the middleware.
 
204 No Content is NOT the correct response to use.
404 is 100% the correct status code to use here.

It is absolutely expected behaviour that `httpResponse.IsSuccessStatusCode` and `httpResponse.EnsureSuccessStatusCode()` fails on a 404.


kind of feel like I am repeating myself, so I am going to stop now :)

use postman
ignore MVC - you MVC implementation is incorrect - fix your MVC implementation. I promise that you are not correct and the rest of the world is wrong
 
What if you are trying to search for a user by email. You return a 404 which is an error when all you want is just positive result of nothing found. Very confusing.
 
datatracker.ietf.org

RFC 7231: Hypertext Transfer Protocol (HTTP/1.1): Semantics and Content

The Hypertext Transfer Protocol (HTTP) is a stateless \%application- level protocol for distributed, collaborative, hypertext information systems. This document defines the semantics of HTTP/1.1 messages, as expressed by request methods, request header fields, response status codes, and response...
datatracker.ietf.org datatracker.ietf.org
 
Solarion said:
What if you are trying to search for a user by email. You return a 404 which is an error when all you want is just positive result of nothing found. Very confusing.
Click to expand...
suppose this is your User
C#: 
public class User
{
    public Guid Id { get; set; } // or int/whatever makes you happy
    public string Firstname { get; set; }
    public string Surname { get; set; }
    public string Email { get; set; }
    public string OtherProperty { get; set; }
}

Available endpoints for users in my application would most likely be
  • GET /api/users - gets a list of users, where any GET parameters provided filters the result set based on the model's properties (id, firstname, surname, email, otherProperty, [or properties on navigations even])
  • GET /api/users/{Id} - gets a specific user
If /api/users/{Id} comes up empty, i.e. no user with that id, I return 404. End of. I don't care whether the platform/framework treats this as an error as this is the proper RestFUL response. (And logging/telemetry/instrumentation can be adjusted if you don't want "errors" on dashboards)

If I am to search for users via email, I'd use /api/users?email=[uri-encoded-user-email-to-cater-for-@-sign]. The /api/users endpoint would include an array containing all the users subject to the GET parameters provided.
For example, take /api/users?surname=smith&limit=10&offset=100, the result would be
JSON: 
{
    offset: 100,
    limit: 10,
    total: 1500,
    users: [
        {
            id: guid,
            firstname: "Adam",
            surname: "Smith",
            email: "[email protected]"
        },
        {
            id: guid,
            firstname: "Adrian",
            surname: "Smith",
            email: "[email protected]"
        },
        ... //others here
        {
            id: guid,
            firstname: "Ben",
            surname: "Smith",
            email: "[email protected]"
        }
    ]
}

Supposing the email you search by comes up empty. The response would be
JSON: 
{
    offset: 0,
    limit: 10,
    total: 0,
    users: []
}
 
Solarion said:
What if you are trying to search for a user by email. You return a 404 which is an error when all you want is just positive result of nothing found. Very confusing.
Click to expand...
It’s REST, so you’re looking up a ‘resource’ by ‘id’. That ‘id’ looks up the resource, as you put it “non-existence” so 404.
A search doesn’t mean the resource doesn’t exist, it just means nothing matched the query. When you use Google it doesn’t take you to a 404 because no results were found.

The reason why you won’t find a lot info on this scenario is because it would never be done or accepted. No one would expose user enumeration by email or something else.

When you use the wrong login, it’s always “user or password is incorrect” they don’t say which, same with password reset “we’ve sent you an email to reset your password”.
There’s no confirmation to the user that account actually exists.

Identity provides those methods under certain assumptions, like email is unique.
Eventually you’d hit a brick wall somewhere with the helper methods, either by trying multi-tenancy or the FindByEmail is creating a SQL query that retrieves all the columns from the table so it’s faster to do the lookup yourself.
 
This is a bit of an interesting article about return 204 results.
DA-LION-619 said:
There’s no confirmation to the user that account actually exists.
Click to expand...

Except on Registration you will most times get "email already in use" which is what I am trying to achieve. My fundamental problem is that in checking if that email exists I get a NotFound return from the API and 404 gets interpreted by the ExceptionMiddleware as an error and I get a 404 page.

The same issue on Login. If the account does not exist, another 404 page.

Think I have painted myself into a corner with the exception middleware.
 
Solarion said:
This is a bit of an interesting article about return 204 results.


Except on Registration you will most times get "email already in use" which is what I am trying to achieve. My fundamental problem is that in checking if that email exists I get a NotFound return from the API and 404 gets interpreted by the ExceptionMiddleware as an error and I get a 404 page.

The same issue on Login. If the account does not exist, another 404 page.

Think I have backed myself into a corner with the exception middleware possibly.
Click to expand...
I thought you were logging :P
What’s the exception?

Registration is a once off, it’s not annoying to a user to enforce recapta or “Please check your inbox to verify your account”, then in the background fire off another email, “Someone tried to signup…., if this was a mistake please ignore”
 
DA-LION-619 said:
I thought you were logging :p
What’s the exception?

Registration is a once off, it’s not annoying to a user to enforce recapta or “Please check your inbox to verify your account”, then in the background fire off another email, “Someone tried to signup…., if this was a mistake please ignore”
Click to expand...

Sec, I'll generate it and send some details here.

Edit: Ok this is in Development mode. I do have a nice pretty page for Production :p

So THIS is what I am bitching about for several days now lol!! Requesting a password reset for an account that does not exist.

Forgot-Password.jpg



C#: 
[HttpGet(ApiRoutes.Users.FindByEmail)]
public async Task<ActionResult<User>> FindByEmail(string email)
{
    var result = await _userManager.FindByEmailAsync(email);

    if(result == null)
    {
        return NotFound();
    }

    return _mapper.Map<User>(result);
}

NotFound.jpg


I can also post the ServiceClient snippet and middleware if you want.
 
Last edited:
One last thing. If I remove the Ensure method I get an empty User

Empty-User.jpg
 
And all Ensure was doing was check if the response is a success which it was not. See my conundrum?


C#: 
public static async Task<T> GetAsync<T>(Uri requestUri)
{
    var response = await client.GetAsync(requestUri);
    var result = await response.Content.ReadAsStringAsync();

    Ensure(response, result);

    T obj = JsonConvert.DeserializeObject<T>(result);

    return obj;
}

C#: 
private static void Ensure(HttpResponseMessage response, string result)
{
    if (response.IsSuccessStatusCode)
        return;

    var responseMessage = "Response status code does not indicate success: " + (int)response.StatusCode + " (" + response.StatusCode + " ). ";
    throw new HttpRequestException(responseMessage + Environment.NewLine + result);
}
 
DA-LION-619 said:
I thought you were logging :p
What’s the exception?

Registration is a once off, it’s not annoying to a user to enforce recapta or “Please check your inbox to verify your account”, then in the background fire off another email, “Someone tried to signup…., if this was a mistake please ignore”
Click to expand...

Nah bru not logging yet. I'll be at this for another hour or so then I'm going to Netflix n popcorn.
 
Last edited:
Solarion said:
One last thing. If I remove the Ensure method I get an empty User

Empty-User.jpg
Click to expand...
That might depend on your JsonConvert global settings, which why you should do your status code check before
await response.Content.ReadAsStringAsync();
BCPlM91rROXWlsSu7TXoH50pazxv9xzFG-wWjg8f3VE.jpg


Solarion said:
And all Ensure was doing was check if the response is a success which it was not. See my conundrum?


C#: 
public static async Task<T> GetAsync<T>(Uri requestUri)
{
    var response = await client.GetAsync(requestUri);
    var result = await response.Content.ReadAsStringAsync();

    Ensure(response, result);

    T obj = JsonConvert.DeserializeObject<T>(result);

    return obj;
}

C#: 
private static void Ensure(HttpResponseMessage response, string result)
{
    if (response.IsSuccessStatusCode)
        return;

    var responseMessage = "Response status code does not indicate success: " + (int)response.StatusCode + " (" + response.StatusCode + " ). ";
    throw new HttpRequestException(responseMessage + Environment.NewLine + result);
}
Click to expand...
But you're throwing the Exception explicitly?
Why not,
if (!response.IsSuccessStatusCode){
return null;
}
before reading the response.Content?
 
DA-LION-619 said:
That might depend on your JsonConvert global settings, which why you should do your status code check before
await response.Content.ReadAsStringAsync();
BCPlM91rROXWlsSu7TXoH50pazxv9xzFG-wWjg8f3VE.jpg



But you're throwing the Exception explicitly?
Why not,
if (!response.IsSuccessStatusCode){
return null;
}
before reading the response.Content?
Click to expand...

That's what I did!! I wasn't sure if I could do that man! I looked at it and just thought it looked all wrong but it worked perfectly. That was like 4 days ago. Then I stripped that check out and down the rabbit hole I tumbled lol!
 
Ok so I'm gonna throw that check back in there on those methods and then make popcorn. Will come back to this tomorrow. My brain is fried now dude.
 
You must log in or register to reply here.
Back
Top
Sign up to the MyBroadband newsletter
X