Skip to content

Conversation

@Narayanbhat166
Copy link
Contributor

Type of Change

  • Enhancement

Description

Provide details on why parsing the struct failed by logging the raw bytes along with the type name which was intended to be converted to.

Motivation and Context

There were many cases where the response handling was failing because of deserialization problem. This will ease up debugging by providing relevant information.

How did you test it?

Create a payment ( or any request where it is sure that it will fail ) and check the logs.
Example of a checkout connector.
Screenshot 2023-02-20 at 5 45 16 PM

Checklist

  • I formatted the code cargo +nightly fmt --all
  • I addressed lints thrown by cargo clippy
  • I reviewed submitted code

@Narayanbhat166 Narayanbhat166 added S-waiting-on-review Status: This PR has been implemented and needs to be reviewed C-refactor Category: Refactor labels Feb 20, 2023
@Narayanbhat166 Narayanbhat166 self-assigned this Feb 20, 2023
@Narayanbhat166 Narayanbhat166 requested a review from a team as a code owner February 20, 2023 12:16

impl<T> BytesExt<T> for bytes::Bytes {
fn parse_struct<'de>(&'de self, type_name: &str) -> CustomResult<T, errors::ParsingError>
fn parse_struct<'de>(&'de self, _type_name: &str) -> CustomResult<T, errors::ParsingError>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to take type from the caller? I think we need to rely on the type system to provide the type name instead of asking the caller to provide, which is prone to error.

@jarnura jarnura removed the S-waiting-on-review Status: This PR has been implemented and needs to be reviewed label Feb 25, 2023
@jarnura jarnura added this pull request to the merge queue Feb 25, 2023
Merged via the queue into main with commit 275155a Feb 25, 2023
@SanchithHegde SanchithHegde deleted the bugfix/attach_bytes_in_parse_struct branch February 25, 2023 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-refactor Category: Refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants