Detect when developer has returned a raw value instead of wrapping it in rep()

[atuttle]

We should be able to detect this situation and throw a useful error message, rather than letting it break at "The getData method was not found."

[y2kiah]

FYI I've made a local modification that allows the developer to safely return raw values (on purpose), by simply detecting the case and wrapping in rep it one level up in the call stack. I could submit a pull request if you're interested, but it's just this:

<!--- If the type returned is not an instance of baseSerializer, wrap it with a call to rep().
    This way we can directly return the object instead of a serializer from resource actions. --->
<cfif NOT isInstanceOf(_taffyRequest.result, "taffy.core.baseSerializer")>
    <cfset local.bean = application._taffy.factory.getBean(_taffyRequest.matchDetails.beanName) />
    <cfset _taffyRequest.result = local.bean.rep(_taffyRequest.result) />
</cfif>

entered immediately before <cfset m.afterResource = getTickCount() /> in core/api.cfc.

I like returning the data type much better for several reasons, which I would be happy to outline.

[atuttle]

@y2kiah that's an interesting approach, thanks for sharing. Part of the reason that it is implemented in its current form was to enable method chaining; so for example you might want to return status 403 to indicate the user isn't authorized, but also some data as to why:

return rep( { "error": "You need the role 'foo' to access this resource" } ).withStatus( 403 );

... But I don't suppose your approach stops anyone from doing that either. It just gives them the option of returning raw data and we'll do the default thing with it in that case.

I like that a lot. Thanks!

[y2kiah]

Right, 401, 403 and others can be handled by throwing a pre-defined exception type in this case, and extra data can be attached to the exception. In the error handler recognize the built-in error types with a "switch" and then handle all others in the default way. That way I have consolidated all error handling to funnel down the exception pipeline, which makes things easier because I can just throw from anywhere, even outside my controller code, and I know the response will be handled correctly.

One benefit you get from this approach is you can strongly type the return type of your controller method, so you get that metadata available through introspection and it can be used to supplement the info in dashboard and documentation.

[y2kiah]

To illustrate, here is some example resource code, followed by my onError handler. Notice how the errors are handled by throwing, and then the response code and data resolves in the error handler.

/**
* ClientResource represents a single ClientModel.
* <p>License level required: Full Access</p>
*/
component displayname="ClientResource"
    extends="resources.BaseResource"
    taffy_uri="/clients/{cliID}"
    Authorize='{ "Policy": "AllAccess" }'
{
    /**
    * Returns a single ClientModel.
    * @_cached "Pass false to bypass caching and force a data refresh. It is recommended that you do not use this option unless you absolutely need up-to-the-minute data, due to the potential degradation of API performance."
    */
    public ClientModel function get(
            required numeric cliID,
            boolean _cached = true,
            required Identity _user  taffy_docs_hide  taffy_dashboard_hide)
    {
        if (cliID <= 0) {
            throw(type = this.ErrorTypes.ResourceNotFoundError);
        }

        var dsn = _user.getClaimValue("agDSN");

        var q = new GetClientsQuery(dsn);
        var data = q.execute(argumentCollection = arguments);

        if (data.recordCount == 0) {
            throw(type = this.ErrorTypes.ResourceNotFoundError);
        }

        var result = this.Mapper.mapFrom(data, "models.Client.ClientModel");

        return result;
    }
...

/**
* Catches all exceptions that go unhandled in application code.
*/
public void function onError(required any Exception, required string EventName)
{
    if (EventName != "onSessionEnd"
        && EventName != "onApplicationEnd")
    {
        var ErrorTypes = new common.ErrorTypes();

        var result = new common.ErrorResult();

        var root = (structKeyExists(Exception, "rootCause"))
                    ? Exception.rootCause
                    : Exception;

        // exceptions thrown by type that match a key in the ErrorTypes enum have their
        // response object defaulted, which can be overidden by exception parameters
        if (structKeyExists(root, "type")
            && ErrorTypes.ErrorResults.keyExists(root.type))
        {
            result = ErrorTypes.ErrorResults[root.type];
        }

        // populate result (or override the defaults) with exception params

        if (structKeyExists(root, "errorCode")
            && isNumeric(root.errorCode))
        {
            result.setStatusCode(root.errorCode);
        }
        else if (isNull(result.getStatusCode())) {
            // return 400 for parameter validation / model binding errors, 500 otherwise
            if (structKeyExists(root, "expectedType")
                || root.stackTrace.left(43) == "coldfusion.runtime.MissingArgumentException")
            {
                result = ErrorTypes.ErrorResults[ErrorTypes.ValidationError];
            }
            else {
                result = ErrorTypes.ErrorResults[ErrorTypes.InternalServerError];
            }
        }

        if (structKeyExists(root, "message")
            && root.message != "")
        {
            if (result.getMessage() == "") {
                result.setMessage(root.message);
            }

            if (!isArray(result.getErrors())) {
                result.setErrors([ root.message ]);
            }
            else {
                result.getErrors().append(root.message);
            }
        }

        // we repurpose this option to either include stack trace information or not, but
        // either way the api is returning a response
        if (APPLICATION._taffy.settings.returnExceptionsAsJson) {
            if (structKeyExists(root, "detail")
                && root.detail != "")
            {
                result.setDetail(root.detail);
            }

            if (structKeyExists(root, "tagContext")) {
                result.setTagContext(root.tagContext[1].template & " [Line #root.tagContext[1].line#]");
            }
        }

        // write the response output
        cfsetting(enablecfoutputonly="true", showdebugoutput="false");
        cfheader(statusCode=result.getStatusCode(), statusText=result.getMessage());
        // TODO: this should serialize based on the Accept header or content type extension requested
        // can I get the bean/serializer and requested mime type from REQUEST scope?
        cfcontent(type="application/json; charset=utf-8", reset=true);

        writeOutput(serializeJson(result));

        // on 500 errors call Taffy's error handler for logging
        if (result.getStatusCode() == 500) {
            super.onError(Exception, EventName);
        }
    }
}