Clarify the serialized wire format for complex error parameters

[ash211]

BLUF: we don't specify the wire format for complex error parameters, and conjure-java and conjure-typescript are behaving differently. We need to determine correct wire format here, and then update the language implementations to match.

---

Here is the current wire spec for error parameters: https://github.com/palantir/conjure/blob/master/docs/spec/wire.md#55-conjure-errors

parameters - a JSON map providing additional information regarding the nature of the error.

Unfortunately, the serialization format for these parameters is under-specified, and there's currently a mismatch between what conjure-java generates, and what conjure-typescript expects. Conjure-java effectively casts complex types to strings and sends the data as a string, whereas conjure-typescript generates .d.ts types expecting the complex types. Instead, it gets strings. This manifests as broken typescript code (at runtime!), and a difficulty accessing complex error parameters from typescript.

Example

Given this yml error definition:

    errors:
      NoAgentsAvailable:
        namespace: AgentAllocation
        code: CUSTOM_CLIENT
        safe-args:
          unhealthySha256: set<string>
          disabledSha256s: map<string, agentInfoService.AgentDisabledInfo>
          nonExistingSha256s: set<string>
        unsafe-args:
          unhealthy: set<AgentId>
          disabled: map<AgentId, agentInfoService.AgentDisabledInfo>
          nonExisting: set<AgentId>
          message: string

The conjure-java library generates errors that look like this (from chrome network tab):

{
    "errorCode": "CUSTOM_CLIENT",
    "errorName": "AgentAllocation:NoAgentsAvailable",
    "errorInstanceId": "682c79a4-b168-4971-acfe-0e58875ad890",
    "parameters": {
        "unhealthySha256": "[]",
        "disabledSha256s": "{}",
        "nonExistingSha256s": "[]",
        "unhealthy": "[]",
        "disabled": "{}",
        "nonExisting": "[]",
        "message": "No healthy and enabled agents available.  Check the status of assigned agents to ensure liveness."
    }
}

Note that the sha256s are strings, and not sets/maps.

The conjure-generated typescript definition expects these to be sets/maps, and not strings:

export interface INoAgentsAvailable {
    'errorCode': "CUSTOM_CLIENT";
    'errorInstanceId': string;
    'errorName': "AgentAllocation:NoAgentsAvailable";
    'parameters': {
        unhealthySha256: Array<string>;
        disabledSha256s: {
            [key: string]: IAgentDisabledInfo;
        };
        nonExistingSha256s: Array<string>;
        unhealthy: Array<string>;
        disabled: {
            [key: string]: IAgentDisabledInfo;
        };
        nonExisting: Array<string>;
        message: string;
    };
}

To handle this, currently FE typescript code is handling both types, outside of the conjure generator.

Sample:

// Conjure types lists of string in errors as string[], but they actually look like this:
// "[ri.foundry.main.dataset.1, ri.foundry.main.dataset.2]"
// So check if they're a string (for forwards-compatibility if it gets fixed) and then parse
// <snip internal issue link...>
export function conjureErrorRidsToRidsList(errorRids: string | string[]) {
    if (Array.isArray(errorRids)) {
        return errorRids;
    }
    // The array isn't valid JSON, we need to strip the square brackets and parse by hand
    const withoutBrackets = errorRids.substring(1, errorRids.length - 1);
    if (withoutBrackets.length === 0) {
        return [];
    }
    try {
        return withoutBrackets.split(", ");
    } catch (e) {
        console.error("conjureErrorRidsToRidsList: Unable to parse rids", e);
        return [];
    }
}

[pkoenig10]

This has been a long standing issue with lots of prior discussion, see palantir/conjure-java#1812.

[ash211]

Looks like we previously thought the wire spec does specify JSON serialization: palantir/conjure-java#1812 (comment) I'll add a clarifying example to the wire format doc.