Feature: Add an option to generate exceptions for Conjure errors#1306
Feature: Add an option to generate exceptions for Conjure errors#1306wsuppiger wants to merge 9 commits into
Conversation
✅ Successfully generated changelog entry!Entry generated via PR titleTo modify this entry, edit PR title using proper format. 📋Changelog Preview✨ Features
|
dtobin
left a comment
There was a problem hiding this comment.
Left a couple more comments.
Separately, it might be helpful to include some code snippets in the PR description of how you expect the generated error classes to be used. This has already diverged from the original issue #910, which is okay. If we want these classes to be useful both on the server (creating) and client (deserializing) side, we should include examples of both.
| emitConstructor(poetWriter); | ||
| emitEncode(poetWriter); | ||
| emitDecode(poetWriter); | ||
|
|
There was a problem hiding this comment.
Should we define the args as properties as well?
| poetWriter.increaseIndent(); | ||
| for (PythonField field : args) { | ||
| poetWriter.writeIndentedLine(String.format( | ||
| "%s=decoder.decode(parameters.get('%s'), %s),", |
There was a problem hiding this comment.
There's some complexity here with how non-string error parameters were serialized historically, see palantir/conjure-java#2608 for a decent explanation. I believe this is still how servers serialize the parameters if you don't provide the Accept-Conjure-Error-Parameter-Format header (or if they don't support it). We should decide how we want to handle this.
There was a problem hiding this comment.
Added support for legacy decoding
| safe-args: | ||
| inputTokenCount: integer | ||
| maxTokens: integer | ||
| docs: The supplied input exceeded the model's maximum context window. |
There was a problem hiding this comment.
This example seemed a little unnecessary so just deleted in favor of the test_decode_recovers_legacy_stringified_scalar_params, can add it back though if you think it is valuable
| poetWriter.increaseIndent(); | ||
| for (PythonField field : args) { | ||
| poetWriter.writeIndentedLine(String.format( | ||
| "%s=decoder.decode(parameters.get('%s'), %s),", |
There was a problem hiding this comment.
Added support for legacy decoding
branch off: #1157
For issue: #910
After this PR
The resulting error looks like:
Conjure:
Python:
Server side — create / serialize:
Client side — deserialize:
Also placed a follow-up item for using the enum for
ErrorCodeafter palantir/conjure-python-client#171.