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

[Merged by Bors] - feat: validate trace payloads to avoid transcript issue (COR-000) #733

Closed
wants to merge 3 commits into from

Conversation

zhihil
Copy link
Contributor

@zhihil zhihil commented Mar 25, 2024

Fixes or implements COR-000

Brief description. What is this change?

Transcript UI is failing because users are able to send debug traces from the Function step without payload.message defined.

We will add validation on the Function step to throw an exception if the user provides a standard trace (e.g. debug) and does not fit the type definition.

Implementation details. How do you make this change?

Added a trace validation utility to ensure that if users are providing traces of a standard type, then their contents conforms to the type definition. If the trace has property field, then

  1. If the field is a built-in property of the interface, then field's value must match the type on the interface
  2. If the field is not a built-in property of the interface, then field's value is not type-checked and the value is simply "passed through"

This approach means that users have the flexibility to define fields that we "don't care about", but must satisfy type safety for when we do.

Setup information

N/A

Deployment Notes

N/A

Related PRs

Checklist

  • Breaking changes have been communicated, including:
    • New required environment variables
    • Renaming of interfaces (API routes, request/response interface, etc)
  • New environment variables have been deployed
  • Appropriate tests have been written
    • Bug fixes are accompanied by an updated or new test
    • New features are accompanied by a new test

@zhihil zhihil self-assigned this Mar 25, 2024
@zhihil zhihil force-pushed the brennan/add-validation-to-payload/COR-000 branch from dd78703 to ae6869c Compare March 25, 2024 15:47
Copy link

sonarcloud bot commented Mar 25, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@zhihil
Copy link
Contributor Author

zhihil commented Mar 26, 2024

bors r+

@bors-vf
Copy link

bors-vf bot commented Mar 26, 2024

👎 Rejected by too few approved reviews

@zhihil
Copy link
Contributor Author

zhihil commented Mar 26, 2024

bors r+

bors-vf bot pushed a commit that referenced this pull request Mar 26, 2024
<!-- You can erase any parts of this template not applicable to your Pull Request. -->

**Fixes or implements COR-000**

### Brief description. What is this change?

<!-- Build up some context for your teammates on the changes made here and potential tradeoffs made and/or highlight any topics for discussion -->

Transcript UI is failing because users are able to send debug traces from the Function step without `payload.message` defined. 

We will add validation on the Function step to throw an exception if the user provides a standard trace (e.g. debug) and does not fit the type definition.
@bors-vf
Copy link

bors-vf bot commented Mar 26, 2024

@bors-vf bors-vf bot changed the title feat: validate trace payloads to avoid transcript issue (COR-000) [Merged by Bors] - feat: validate trace payloads to avoid transcript issue (COR-000) Mar 26, 2024
@bors-vf bors-vf bot closed this Mar 26, 2024
@bors-vf bors-vf bot deleted the brennan/add-validation-to-payload/COR-000 branch March 26, 2024 16:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants