Skip to content

Conversation

@justinkambic
Copy link
Contributor

@justinkambic justinkambic commented Mar 19, 2025

Summary

Right now we return a 404 anytime that the data we're looking for on the screenshot_ref route is not satisfactory. We do an io-ts check on the data before returning. It's possible that that data will fail the check, and we'd return a 404 anyway. This isn't a very accurate reflection of what's happening on the server, and could indicate a problem with the user's data.

Instead, we first check if the data returned from Elasticsearch is null, and if it is we return a 404. Otherwise, we compute the type check like normal and return the result. In the case where the data fails the type check, we instead return a 500 and include the malformed data in the server response.

@justinkambic justinkambic added release_note:enhancement v9.0.0 backport:prev-minor Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. labels Mar 19, 2025
@justinkambic justinkambic self-assigned this Mar 19, 2025
@justinkambic justinkambic requested a review from a team as a code owner March 19, 2025 20:06
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-management-team (Team:obs-ux-management)

Copy link
Contributor

@fkanout fkanout left a comment

Choose a reason for hiding this comment

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

code review only, LGTM

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #66 / EQL execution logic API @ess @serverless @serverlessQA EQL type rules parses shard failures for EQL event query

Metrics [docs]

✅ unchanged

History

cc @justinkambic

Copy link
Contributor

@dominiqueclarke dominiqueclarke left a comment

Choose a reason for hiding this comment

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

LGTM

@justinkambic justinkambic merged commit 74f87d9 into elastic:main Mar 24, 2025
9 checks passed
@justinkambic justinkambic deleted the screenshot_ref_route_only_404_when_data_missing branch March 24, 2025 14:50
@kibanamachine
Copy link
Contributor

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Mar 24, 2025
…elastic#215241)

## Summary

Right now we return a 404 anytime that the data we're looking for on the
`screenshot_ref` route is not satisfactory. We do an io-ts check on the
data before returning. It's possible that that data will fail the check,
and we'd return a 404 anyway. This isn't a very accurate reflection of
what's happening on the server, and could indicate a problem with the
user's data.

Instead, we first check if the data returned from Elasticsearch is
`null`, and if it is we return a 404. Otherwise, we compute the type
check like normal and return the result. In the case where the data
fails the type check, we instead return a 500 and include the malformed
data in the server response.

Co-authored-by: Faisal Kanout <[email protected]>
(cherry picked from commit 74f87d9)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
9.0

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

JoseLuisGJ pushed a commit to JoseLuisGJ/kibana that referenced this pull request Mar 24, 2025
…elastic#215241)

## Summary

Right now we return a 404 anytime that the data we're looking for on the
`screenshot_ref` route is not satisfactory. We do an io-ts check on the
data before returning. It's possible that that data will fail the check,
and we'd return a 404 anyway. This isn't a very accurate reflection of
what's happening on the server, and could indicate a problem with the
user's data.

Instead, we first check if the data returned from Elasticsearch is
`null`, and if it is we return a 404. Otherwise, we compute the type
check like normal and return the result. In the case where the data
fails the type check, we instead return a 500 and include the malformed
data in the server response.

Co-authored-by: Faisal Kanout <[email protected]>
kibanamachine added a commit that referenced this pull request Mar 24, 2025
…resent (#215241) (#215726)

# Backport

This will backport the following commits from `main` to `9.0`:
- [[Synthetics] Only return 404 if `screenshot_ref` is truly not present
(#215241)](#215241)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Justin
Kambic","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-03-24T14:50:18Z","message":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not present (#215241)\n\n##
Summary\n\nRight now we return a 404 anytime that the data we're looking
for on the\n`screenshot_ref` route is not satisfactory. We do an io-ts
check on the\ndata before returning. It's possible that that data will
fail the check,\nand we'd return a 404 anyway. This isn't a very
accurate reflection of\nwhat's happening on the server, and could
indicate a problem with the\nuser's data.\n\nInstead, we first check if
the data returned from Elasticsearch is\n`null`, and if it is we return
a 404. Otherwise, we compute the type\ncheck like normal and return the
result. In the case where the data\nfails the type check, we instead
return a 500 and include the malformed\ndata in the server
response.\n\nCo-authored-by: Faisal Kanout
<[email protected]>","sha":"74f87d99bc1981a533982e103f802d0cbf118fa7","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","backport:prev-minor","Team:obs-ux-management","v9.1.0"],"title":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not
present","number":215241,"url":"https://github.com/elastic/kibana/pull/215241","mergeCommit":{"message":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not present (#215241)\n\n##
Summary\n\nRight now we return a 404 anytime that the data we're looking
for on the\n`screenshot_ref` route is not satisfactory. We do an io-ts
check on the\ndata before returning. It's possible that that data will
fail the check,\nand we'd return a 404 anyway. This isn't a very
accurate reflection of\nwhat's happening on the server, and could
indicate a problem with the\nuser's data.\n\nInstead, we first check if
the data returned from Elasticsearch is\n`null`, and if it is we return
a 404. Otherwise, we compute the type\ncheck like normal and return the
result. In the case where the data\nfails the type check, we instead
return a 500 and include the malformed\ndata in the server
response.\n\nCo-authored-by: Faisal Kanout
<[email protected]>","sha":"74f87d99bc1981a533982e103f802d0cbf118fa7"}},"sourceBranch":"main","suggestedTargetBranches":["9.0"],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/215241","number":215241,"mergeCommit":{"message":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not present (#215241)\n\n##
Summary\n\nRight now we return a 404 anytime that the data we're looking
for on the\n`screenshot_ref` route is not satisfactory. We do an io-ts
check on the\ndata before returning. It's possible that that data will
fail the check,\nand we'd return a 404 anyway. This isn't a very
accurate reflection of\nwhat's happening on the server, and could
indicate a problem with the\nuser's data.\n\nInstead, we first check if
the data returned from Elasticsearch is\n`null`, and if it is we return
a 404. Otherwise, we compute the type\ncheck like normal and return the
result. In the case where the data\nfails the type check, we instead
return a 500 and include the malformed\ndata in the server
response.\n\nCo-authored-by: Faisal Kanout
<[email protected]>","sha":"74f87d99bc1981a533982e103f802d0cbf118fa7"}}]}]
BACKPORT-->

Co-authored-by: Justin Kambic <[email protected]>
cqliu1 pushed a commit to cqliu1/kibana that referenced this pull request Mar 31, 2025
…elastic#215241)

## Summary

Right now we return a 404 anytime that the data we're looking for on the
`screenshot_ref` route is not satisfactory. We do an io-ts check on the
data before returning. It's possible that that data will fail the check,
and we'd return a 404 anyway. This isn't a very accurate reflection of
what's happening on the server, and could indicate a problem with the
user's data.

Instead, we first check if the data returned from Elasticsearch is
`null`, and if it is we return a 404. Otherwise, we compute the type
check like normal and return the result. In the case where the data
fails the type check, we instead return a 500 and include the malformed
data in the server response.

Co-authored-by: Faisal Kanout <[email protected]>
@justinkambic
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

justinkambic added a commit to justinkambic/kibana that referenced this pull request Apr 14, 2025
…elastic#215241)

## Summary

Right now we return a 404 anytime that the data we're looking for on the
`screenshot_ref` route is not satisfactory. We do an io-ts check on the
data before returning. It's possible that that data will fail the check,
and we'd return a 404 anyway. This isn't a very accurate reflection of
what's happening on the server, and could indicate a problem with the
user's data.

Instead, we first check if the data returned from Elasticsearch is
`null`, and if it is we return a 404. Otherwise, we compute the type
check like normal and return the result. In the case where the data
fails the type check, we instead return a 500 and include the malformed
data in the server response.

Co-authored-by: Faisal Kanout <[email protected]>
(cherry picked from commit 74f87d9)
justinkambic added a commit that referenced this pull request Apr 14, 2025
…resent (#215241) (#218136)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Synthetics] Only return 404 if `screenshot_ref` is truly not present
(#215241)](#215241)

<!--- Backport version: 9.6.6 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Justin
Kambic","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-03-24T14:50:18Z","message":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not present (#215241)\n\n##
Summary\n\nRight now we return a 404 anytime that the data we're looking
for on the\n`screenshot_ref` route is not satisfactory. We do an io-ts
check on the\ndata before returning. It's possible that that data will
fail the check,\nand we'd return a 404 anyway. This isn't a very
accurate reflection of\nwhat's happening on the server, and could
indicate a problem with the\nuser's data.\n\nInstead, we first check if
the data returned from Elasticsearch is\n`null`, and if it is we return
a 404. Otherwise, we compute the type\ncheck like normal and return the
result. In the case where the data\nfails the type check, we instead
return a 500 and include the malformed\ndata in the server
response.\n\nCo-authored-by: Faisal Kanout
<[email protected]>","sha":"74f87d99bc1981a533982e103f802d0cbf118fa7","branchLabelMapping":{"^v9.1.0$":"main","^v8.19.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement","v9.0.0","backport:prev-minor","Team:obs-ux-management","v9.1.0"],"title":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not
present","number":215241,"url":"https://github.com/elastic/kibana/pull/215241","mergeCommit":{"message":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not present (#215241)\n\n##
Summary\n\nRight now we return a 404 anytime that the data we're looking
for on the\n`screenshot_ref` route is not satisfactory. We do an io-ts
check on the\ndata before returning. It's possible that that data will
fail the check,\nand we'd return a 404 anyway. This isn't a very
accurate reflection of\nwhat's happening on the server, and could
indicate a problem with the\nuser's data.\n\nInstead, we first check if
the data returned from Elasticsearch is\n`null`, and if it is we return
a 404. Otherwise, we compute the type\ncheck like normal and return the
result. In the case where the data\nfails the type check, we instead
return a 500 and include the malformed\ndata in the server
response.\n\nCo-authored-by: Faisal Kanout
<[email protected]>","sha":"74f87d99bc1981a533982e103f802d0cbf118fa7"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"9.0","label":"v9.0.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/215726","number":215726,"state":"MERGED","mergeCommit":{"sha":"349e4472ee62edd72cd062bb2635cb563f61168b","message":"[9.0]
[Synthetics] Only return 404 if `screenshot_ref` is truly not present
(#215241) (#215726)\n\n# Backport\n\nThis will backport the following
commits from `main` to `9.0`:\n- [[Synthetics] Only return 404 if
`screenshot_ref` is truly not
present\n(#215241)](https://github.com/elastic/kibana/pull/215241)\n\n\n\n###
Questions ?\nPlease refer to the [Backport
tool\ndocumentation](https://github.com/sorenlouv/backport)\n\n\n\nCo-authored-by:
Justin Kambic
<[email protected]>"}},{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/215241","number":215241,"mergeCommit":{"message":"[Synthetics]
Only return 404 if `screenshot_ref` is truly not present (#215241)\n\n##
Summary\n\nRight now we return a 404 anytime that the data we're looking
for on the\n`screenshot_ref` route is not satisfactory. We do an io-ts
check on the\ndata before returning. It's possible that that data will
fail the check,\nand we'd return a 404 anyway. This isn't a very
accurate reflection of\nwhat's happening on the server, and could
indicate a problem with the\nuser's data.\n\nInstead, we first check if
the data returned from Elasticsearch is\n`null`, and if it is we return
a 404. Otherwise, we compute the type\ncheck like normal and return the
result. In the case where the data\nfails the type check, we instead
return a 500 and include the malformed\ndata in the server
response.\n\nCo-authored-by: Faisal Kanout
<[email protected]>","sha":"74f87d99bc1981a533982e103f802d0cbf118fa7"}}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:enhancement Team:actionable-obs Formerly "obs-ux-management", responsible for SLO, o11y alerting, significant events, & synthetics. v8.19.0 v9.0.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants