Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Que3216 opened this issue Jul 1, 2022 · 2 comments

Comments

@Que3216
Copy link

Que3216 commented Jul 1, 2022

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
Copy link

ash211 commented Jul 18, 2022

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
Copy link

ash211 commented Jul 18, 2022

Filed palantir/conjure#1217 to clarify wire format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants