Best practice for returning non-500 error in an HTTP service

[manuranga]

Goal

Following scenario has come up during the internal apps project. We need to determine a general best practice for where to create non-500 error in an HTTP service.

Issue

service / {
  resource function GET promotion-cycle/[string cycle]/candidates/[string id] returns UserPromotionStatus|UserNotFound {
    ...
    User user = check usersFromId(id);
    ...
    return { isEligible, requestSubmitted, ...  };
  }
}

function userFromId(string id) returns User|UserNotFound {
    ...
}

Above solution produces 500 when the user doesn't exist, but let's say the programmer intends to return HTTP 404. Issue gets worse if there are multiple levels of function calls before we get to usersFromId. How can one achieve this?

I think the root issue is the inability to use the check keyword with HTTP errors.

Proposed solution 1

Create a function that maps internal errors to http errors. Wrap returns in resource functions with a call to the function.

public isolated function respond(anydata|error reply ) returns http:Ok|http:Unauthorized|http:InternalServerError|http:NotFound {
  if reply is PromotionAppError {
    // create an http error, pick type using reply.code
  }
  else if reply is error {
    // return 500 HTTP error
  }
  else {
    // return 200 OK with reply as data
  }
}

But this solution negatively impacts the diagram generation, as well as understandability.

Proposed solution 2

Same as above solution, but move the logic in the function to an interceptor. This cleans up the resource function.
But this adds an unnecessary layer to the http logic.

Proposed solution 3

Create http errors at deeper levels.

service / {
  resource function GET promotion-cycle/[string cycle]/candidates/[string id] returns UserPromotionStatus|http:NotFound {
    ...
    User|http:NotFound userResult = usersFromId(id);
    if userResult !is User {
      return userResult;
    }
    ...
    return { isEligible, requestSubmitted ....  };
  }
}

function userFromId(string id) returns User|http:NotFound {
    ...
}

Ugly since usersFromId knows about HTTP, and can't use the check keyword.

Solution 4

service / {
  resource function GET promotion-cycle/[string cycle]/candidates/[string id] returns UserPromotionStatus|UserNotFound {
    ...
    User|UserNotFound userResult = usersFromId(id);
    if userResult !is User {
      // create and return 404 HTTP error
    }
    ...
    return { isEligible, requestSubmitted .. .  };
  }
}

function userFromId(string id) returns User|UserNotFound {
    ...
}

This seems to me the simplest, even if it complicates resources a little bit.

[jclark]

A "resource does not exist" error makes sense at the Ballerina language level, independent of HTTP. This feels like a common case. One could handle this by having langlib defining a distinct error type for this, which HTTP could map into 404.

Apart from this, what are the common cases where application wants to return a non-500 error?

[manuranga]

They have:
400 Bad Request - Due to email/date/time validation
401 Unauthorized - Custom role validation, maybe these should go away with proper resource level jwt scope validation.
403 Forbidden - Rejected due to business logic (eg: user banned form certification process due to prior misuse)

[manuranga]

We are also seeing 501 Not Implemented.

[manuranga]

We discussed that 501 is inappropriate to indicate given API according to the spec. It means the given http method is not supported for any resources.

[manuranga]

Another requirement:
409 Conflict - New mapping can't be created due to pre-existing mapping. Caller has to delete the existing mapping first.

[manuranga]

https://learn.microsoft.com/en-us/rest/api/storageservices/common-rest-api-error-codes
is a good resource to see commonly used http codes.

[manuranga]

@jclark @shafreenAnfar
Can http:NotFound be a subtype of error? If that was the case programmers can create UserNotFound to be a subtype of http:NotFound.

[manuranga]

Discussed in high level chat:
We should recommend people to mostly return only 500 errors from user code. Majority of the non-500 errors should come from the http engine.
Yet to come up with a solution for edge cases.

[manuranga]

Following idea was proposed for edge cases by James (names are my own):

// in lang.error
public type NotFound distinct error;

type UserNotFound distinct error:NotFound;

function userFromId(string id) returns User|UserNotFound {
    ...
}

http library will check if the returned error is a subtype of lang:NotFound and return 404.

[manuranga]

We discussed having another error similar to above for BadRequests.

[jclark]

I see one anti-pattern in this:

https://ballerina.io/learn/by-example/http-error-handling/

which is to define an object type with a single method. This feels like a Java-ism stemming from OO thinking plus absence of a function type.

I would say that it makes better use of the language to use a function type instead (in this case from a error to an HTTP response).

[manuranga]

Users mentioned that when the api are called by websites, they need to present user friendly error messages for display purposes.