ecyrbe / zodios

typescript http client and server with zod validation
https://www.zodios.org/
MIT License
1.71k stars 46 forks source link

z.transform() on errors #399

Closed Zehir closed 1 year ago

Zehir commented 1 year ago

Hello,

It's look like the transform method is not called when coming from errors calls.

For example


  makeEndpoint({
    alias: 'updateConfig',
    description: 'Modify configuration parameters.',
    method: 'put',
    path: '/api/:apiKey/config',
    response: writableConfigSchema,
    parameters: [
      parameters.apiKey,
      {
        name: 'body',
        type: 'Body',
        schema: writableConfigSchema,
      },
    ],
    errors: [
      {
        status: 400,
        description: 'OK: Request succeded.',
        schema: z.unknown().transform((foo) => {
          console.log(foo)
          return foo
        }),
      },
    ],

    // getErrors(400, 403), // TODO check code with errors code
  }),

The console.log is not called and I get a AxiosError exception with status:400 so it's should work right ?

Thanks

ecyrbe commented 1 year ago

Hello,

Thank you for the report, but this behaviour is normal. There is no way to do transforms on a failing schema or response error. If you need to transform errors, use zodios plugins

Zehir commented 1 year ago

But how can I find what api call make that request in the plugin ? I have the api params with all api but I can't find the correct using only the request method and the path, right ? I may have multiple matching result

ecyrbe commented 1 year ago

Plugins pass the config as meta information. You have everything to handle your error. zod transforms are not there to log errors or catch them, but validate and transform validated data

ecyrbe commented 1 year ago

You can attach a plugin to one endpoint only btw

Zehir commented 1 year ago

Plugins pass the config as meta information. You have everything to handle your error. zod transforms are not there to log errors or catch them, but validate and transform validated data

Yes but to correctly modify the result I need to have the expected schema as it's will change the method to parse the response;

The rest api send me errors result like this :

[
  { 
    "success": {"/config/zigbeechannel": 25 }
  }, 
 { 
    "success": {"/config/foo": "bar"}
  }, 
 { 
    "errors": {"/config/enabled": true}
  }
]

But it's not practical to read and I would like to parse it to :

{
  "success" : {
    "zigbeechannel" : 25
    "foor" : "bar"
  }
  "errors" : {
      "enabled" : true
  }
}

But in the endpoint configuration I just set the inner schema as the outer is fixed for all endpoint.

In the config meta I don"t find the alias of the endpoint ?

With 50+ endpoint it's will not be laggy to have the same plugin registred for each endpoint ?

Thanks for your help

Zehir commented 1 year ago

OK I found the findEndpoint method in the original zod plugin

Zehir commented 1 year ago

Is it possible for you to add the findEndpoint in the export ?

ecyrbe commented 1 year ago

I prefer not. It's a simple one line find. It should not be an issue to duplicate this on your code.

Zehir commented 1 year ago

I am still confused by this.

I made a plugin to unwrap the data :

https://github.com/deconz-community/rest-client/blob/main/packages/rest-client/src/gateway/plugins.ts#L62-L181

It's to transform the api http result (that I can't change because I did not make it):

[
  { 
    "success": {"/config/zigbeechannel": 25 }
  }, 
 { 
    "success": {"/config/foo": "bar"}
  }, 
 { 
    "errors": {"/config/enabled": true}
  }
]

Into a more friendly format

{
  "success" : {
    "zigbeechannel" : 25
    "foor" : "bar"
  }
  "errors" : {
      "enabled" : true
  }
}

When there is errors and the success content if there is no error.

For users using the client it's will be more easier to use the data unwrapped. That why I change the format in the client.

My issue is now that the client plugin is linked to the schema. Without it it's can't work. Because if the response is not unwrapped the schema is incorrect.

But you saied in #410 that it's should probably not by like that.

It's look like the correct approach will be to transform the endpoint with a unwrap method but in that case I will need to be able to use transform on error responses too.

Is there a better way to handle that in V11 ?

What should I do with V10 ?

I am starting the app so it's could be worth migrating on V11 beta now and avoid a refactory in near future. By time my client is ready the V11 should be probably released.

Thanks

ecyrbe commented 1 year ago

If you want to transform errors, you just disable error throwing behaviour and handle errors manually : see validate status in requests options https://www.zodios.org/docs/client#request-options

Zehir commented 1 year ago

It's look like I can have multiple errors deifnition with the same status code. Is that expected ?

ecyrbe commented 1 year ago

If you want to validate errors with zod when disabling error throwing, you'll need to handle error as response type. Error status definition will be looked at. Error definition multiplication for same status code is Indeed possible and was a feature requested

Zehir commented 1 year ago

If you want to validate errors with zod when disabling error throwing, you'll need to handle error as response type. Error status definition will be looked at.

I am not sure to understand this correctly, does this mean I need to have a response schema that handle both the success and the error data ? Or It's will still process the errors with the error schema ?

Error definition multiplication for same status code is Indeed possible and was a feature requested

Ok

Andyloz commented 1 year ago

Axios throwing errors like that and not being manageable with Zodios is becoming a pain...

I have the same problem. IMHO it should be transformed by Zodios. Why does it validate but not transform? It doesn't make sense.

ecyrbe commented 1 year ago

Errors are not transformable. Disable errors if you want to transform results. Transforms are for happy path.

Andyloz commented 1 year ago

Okk. Is there any way to have more than one type of response, each by status code as errors?

ecyrbe commented 1 year ago

Of course, just use zod unions.

Zehir commented 1 year ago

@Andyloz For now I use this solution, I don't like it but it's working : https://github.com/deconz-community/rest-client/blob/main/packages/rest-client/src/gateway/endpoints/configEndpoints.ts

And I say axios to close they eye on the status code : https://github.com/deconz-community/rest-client/blob/7d6a993636f278cb125dce72ec21ac6b4dd5bbd7/packages/rest-client/src/gateway/index.ts#L26