palantir / conjure-typescript

Conjure generator for TypeScript clients
Apache License 2.0
17 stars 16 forks source link

Generates invalid typings for ServiceExceptions with non primitive param types #217

Open Que3216 opened 2 years ago

Que3216 commented 2 years ago

What happened?

   InvalidTimeIntervals:
        namespace: MyService
        code: INVALID_ARGUMENT
        safe-args:
          invalidTimeIntervals: set<TimeInterval>

Generates the typings:

import { ITimeInterval } from "./timeInterval";
export interface IInvalidTimeIntervals {
    'errorCode': "INVALID_ARGUMENT";
    'errorInstanceId': string;
    'errorName': "MyService:InvalidTimeIntervals";
    'parameters': {
        invalidTimeIntervals: Array<ITimeInterval>;
    };
}
export declare function isInvalidTimeIntervals(arg: any): arg is IInvalidTimeIntervals;

But at runtime generates the value:

{
        "errorCode": "INVALID_ARGUMENT",
        "errorName": "MyService:InvalidTimeIntervals",
        "errorInstanceId": "....",
        "parameters": {
            "invalidTimeIntervals": "[TimeInterval{start: DayTime{day: MONDAY, time: 10:00}, end: DayTime{day: MONDAY, time: 10:01}}]",
        }
    }

Note that invalidTimeIntervals is serialized as a string (not even valid JSON). This mismatch can cause bugs at runtime if the developer does not realize the typings are incorrect.

What did you want to happen?

It to generate typings of:

import { ITimeInterval } from "./timeInterval";
export interface IInvalidTimeIntervals {
    'errorCode': "INVALID_ARGUMENT";
    'errorInstanceId': string;
    'errorName': "MyService:InvalidTimeIntervals";
    'parameters': {
        invalidTimeIntervals: string; // <<<<<
    };
}
export declare function isInvalidTimeIntervals(arg: any): arg is IInvalidTimeIntervals;

Or the Conjure BE to not serialize params, or serialize them to JSON for the FE to parse (though expect this would be a hard to make break).

ash211 commented 2 years ago

I'm encountering the same issue. Is the expected wire serialization format specified for complex conjure error parameters? https://github.com/palantir/conjure/blob/master/docs/spec/wire.md#55-conjure-errors

ash211 commented 2 years ago

Filed https://github.com/palantir/conjure/issues/1217 to clarify wire format.