Repository navigation
fix(ci): accept 204 from the o11y deploy-event route - #379
Merged
Merged
Conversation
The route answers 204 on success, so the '!= 200' check warned on every healthy deploy event. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
The three deploy-event steps in
master.ymlchecked for200, butPOST /telemetry/deployanswers204on success (respondIngestedinrunner/workers/o11y/src/normalise/respond.tsreturnsnew Response(null, { status: 204 })after the event is written to the inbox). Run 36553451764 therefore emitted a falsedid not return 200 (204)warning forhandsontable-demos-o11yandhandsontable-demos-api. The steps now treat204(and200) as success and warn on anything else; the docs note the real success code. Separately, the same run'shandsontable-demos-authoringstep got405because the/telemetry/*route was not attached yet on that first deploy; that is a real miss and is deliberately not touched here.Types of changes
runner/) changeHow was this verified?
Read the handler (
handleDeploy->inboxWriter.ingest->respondIngested-> 204) and the run logs for 36553451764 (deploy-o11yanddeploy-apiboth loggeddeploy-event response: 204,deploy-authoringlogged405).actionlinton the workflows is clean. No worker code changed, so no runner tests are affected;pipeline/o11y-routes.test.mjsalready asserts any 2xx for this route.Checklist
runner/config/frameworks.json(see CONTRIBUTING.md); otherwise it won't appear on demos.handsontable.com (n/a, no example changed)pnpm build(andpnpm dev) in the affected example/server-example locally (n/a, workflow and docs only)Related issue(s):
Note
Low Risk
Workflow and documentation only; no runtime or auth changes—just correct HTTP success checks for deploy telemetry reporting.
Overview
Aligns master deploy jobs with the real
POST /telemetry/deploycontract: success is204fromrespondIngested, but the three post-deploy “report deploy to o11y” steps inmaster.ymlonly treated200as OK, so successful runs logged false warnings (e.g.did not return 200 (204)).Each step now accepts
204or200and warns on any other status;runner/docs/run-and-deploy.mddocuments204as the success code for that optional, non-failing reporting step.Reviewed by Cursor Bugbot for commit c3090c7. Bugbot is set up for automated code reviews on this repo. Configure here.