docs: Update attachment header requirements in documentation#17040
docs: Update attachment header requirements in documentation#17040tobias-wilfert merged 2 commits intomasterfrom
Conversation
While implementing this in Relay noticed that `ChunkedAttachment` has a `attachment_type` field which we want to set with the original file's attachment type.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Co-authored-by: Joris Bayer <joris.bayer@sentry.io>
| | `filename` | String | **REQUIRED** | The name of the uploaded file without a path component. | | ||
|
|
||
| The item header **MAY** also contain `attachment_type` and the original file's `content_type` as an additional field (see [Standard Attachment Item](#standard-attachment-item)). | ||
| The item header **MUST** also contain the original file's `attachment_type` if it was set. The item payload **MUST** contain the original file's `content_type` as a field in the JSON object (see [Standard Attachment Item](#standard-attachment-item)). |
There was a problem hiding this comment.
Bug: The documentation introduces contradictions. Prose requires attachment_type in the header and content_type in the payload, but the corresponding tables and examples mark them as missing or optional.
Severity: MEDIUM
Suggested Fix
Update the documentation to be consistent. Either modify the prose to align with the tables and examples, or update the tables and examples to reflect the MUST requirements for attachment_type in the item header and content_type in the item payload. For example, add attachment_type to the item headers table and change content_type in the payload table from OPTIONAL to REQUIRED.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: develop-docs/sdk/telemetry/attachments.mdx#L258
Potential issue: The documentation change introduces two contradictions that will likely
cause interoperability issues between different SDK and Relay implementations. First,
the prose at line 258 states the item header `MUST` contain `attachment_type` if set,
but the corresponding item header table and the provided example omit this field.
Second, the prose states the item payload `MUST` contain `content_type`, but the payload
field table marks this field as `OPTIONAL`. These inconsistencies will lead to divergent
implementations, potentially causing problems with how attachments are processed.
Did we get this right? 👍 / 👎 to inform future reviews.
| | `filename` | String | **REQUIRED** | The name of the uploaded file without a path component. | | ||
|
|
||
| The item header **MAY** also contain `attachment_type` and the original file's `content_type` as an additional field (see [Standard Attachment Item](#standard-attachment-item)). | ||
| The item header **MUST** also contain the original file's `attachment_type` if it was set. The item payload **MUST** contain the original file's `content_type` as a field in the JSON object (see [Standard Attachment Item](#standard-attachment-item)). |
There was a problem hiding this comment.
Maybe this will make sentry bot happy:
| The item header **MUST** also contain the original file's `attachment_type` if it was set. The item payload **MUST** contain the original file's `content_type` as a field in the JSON object (see [Standard Attachment Item](#standard-attachment-item)). | |
| The item header **MUST** also contain the original file's `attachment_type` if it was set. The item payload **MUST** contain the original file's `content_type` as a field in the JSON object if it was set (see [Standard Attachment Item](#standard-attachment-item)). |
While implementing this in Relay noticed that
ChunkedAttachmenthas aattachment_typefield which we want to set with the original file's attachment type.