Skip to content

Conversation

Korbeil
Copy link
Member

@Korbeil Korbeil commented Jan 6, 2023

Fixes #682

Adds:

@Korbeil Korbeil force-pushed the feature/issue-682 branch 2 times, most recently from d57ba10 to e38acc5 Compare January 6, 2023 13:37
@LoicBoursin
Copy link

We encountered the same issue as #682 in our project for 400 Bad Request responses. I manually made it work for on my project and was about to create a PR on Jane but I just saw yours haha

Your PR is on the road to fix the issue, but it's fixing only a part of the issue. You are adding the response as a parameter in transformResponseBody but you stopped there. You also need to pass this exception as a parameter to all exceptions like

throw new \AppGenerated\Api\MonolithInternal\Exception\ChangeStatusInternalAdItemBadRequestException();

becomes

throw new \AppGenerated\Api\MonolithInternal\Exception\ChangeStatusInternalAdItemBadRequestException($response);

and then inside each exception class, add the new $response as an attribute so we can retrieve it.

@Korbeil Korbeil force-pushed the feature/issue-682 branch 2 times, most recently from 9cbccd6 to a4f4ee1 Compare January 7, 2023 17:36
@Korbeil
Copy link
Member Author

Korbeil commented Jan 7, 2023

@LoicBoursin I just fixed the issue you mentioned, could you make a second pass ? 🙏

@LoicBoursin
Copy link

Thanks so much, I'll test it on monday 09/01 morning and let you know if it fixes the issue ASAP.

@LoicBoursin
Copy link

@Korbeil Looking at the generated files in the tests folder, I think it will work.

But it looks like you only changed Generator/Endpoint/GetTransformResponseBodyTrait.php. Are you sure you committed all the updated files you used to generated these new fixtures ?

By applying your changes to that file on my project locally, I can't (as expected) get it to work.

@Korbeil
Copy link
Member Author

Korbeil commented Jan 9, 2023

@LoicBoursin I just checked locally and I have nothing more to commit 🤔
Also, I can't find the OpenApiCommon files but when I'm using the .diff extension on the PR, I can find them at the very end of the diff file:
image

Maybe it's a limit that Github have and I have too much files / changes ?

@LoicBoursin
Copy link

@Korbeil Looking at the patch files, the PR is correct. GitHub may have an issue displaying so much changes, and the patch file should be the truth rather than GitHub's interface.

So I targeted your PR directly and installed it locally, generated our files & schema and I can confirm that I now have access to the real parent error message sent from our other client ! 🎉

It seems you just have left a little issue with your tests because of use statements ordering 😃

@Korbeil
Copy link
Member Author

Korbeil commented Jan 12, 2023

It seems you just have left a little issue with your tests because of use statements ordering

Yeah, I have some issues with php-cs-fixer (it seems there is a different version between Jane one and generated code one), I need to check that later.

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

Successfully merging this pull request may close these issues.

Impossible to read response body for a failed request
2 participants