GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [connectors] Report "no Samply profile" as 204, not as an error by blp · Pull Request #7934 · feldera/feldera

Conversation

@blp

@blp blp commented Nov 11, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

When the Samply tab opens, the web console checks for a profile with GET /samply_profile?latest=true. Before any profile exists, the pipeline answered 400 NoSamplyProfile. In feldera/cloud#2163 those 400s in the browser network log sent the support bundle investigation the wrong way.

Request Before After
?latest=true, profile in progress 204 + Retry-After same
?latest=true, no profile 400 NoSamplyProfile 204, no Retry-After
no latest, no profile 400 NoSamplyProfile same
  • The web console reads a 204 without Retry-After as "no profile".
  • The API docs said 307 for a profile in progress. The code sends 204, so the docs now say 204.
  • Changelog entry added, because this is an API change.

Multihost needs feldera/cloud#2207: the coordinator read a host's 204 as an empty profile.

Tests

  • New samply_profile_latest_without_profile engine test. It fails (400 instead of 204) with the fix disabled.
  • svelte-check: 0 errors. Prettier: clean.

-- Claude Code, at @blp's request.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@blp
[connectors] Report "no Samply profile" as 204, not as an error.
8a01e5d
When the Samply tab opens, the web console checks for a profile with
`GET /samply_profile?latest=true`. Before any profile, the pipeline
answered 400 `NoSamplyProfile`, so each check put an error in the
browser's network log. In feldera/cloud#2163, these errors sent an
investigation of a failed support bundle the wrong way.

With `latest=true`, a 204 No Content now means that there is no profile
to download yet: with `Retry-After` if one is in progress, as before,
and without it if none was started. Without `latest`, nothing changes.

Also corrects the API docs, which said 307 for a profile in progress.

Signed-off-by: Ben Pfaff <[email protected]>
@blp blp added bug Something isn't working Web Console Related to the browser based UI API Distributed system APIs connectors Issues related to the adapters/connectors crate profiler Issues related to the profiler and its APIs labels Nov 11, 2026
@blp blp self-assigned this Nov 11, 2026
mythical-fred-oss[bot]
mythical-fred-oss Bot reviewed Nov 11, 2026
View reviewed changes
Comment thread js-packages/web-console/src/lib/services/pipelineManager.ts
const expectedInSeconds = parseInt(response.headers.get('Retry-After') ?? '')
const retryAfter = response.headers.get('Retry-After')
if (retryAfter === null) {
return null

@mythical-fred-oss mythical-fred-oss Bot Nov 11, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

missing (medium): this new 204 branch is the client half of the contract, and no test covers it. The Rust test only checks the server. Here is a unit test I ran locally with vitest --project server (5/5 pass). It could go in as src/lib/services/samplyProfile.spec.ts:

import { afterEach, describe, expect, it, vi } from 'vitest'
import { getSamplyProfile } from '#lib/services/pipelineManager.ts'

const stubFetch = (response: Response) =>
  vi.stubGlobal('fetch', Object.assign(vi.fn().mockResolvedValue(response), { preconnect: () => {} }))

describe('getSamplyProfile', () => {
  afterEach(() => vi.unstubAllGlobals())

  it('returns null on 204 without Retry-After', async () => {
    stubFetch(new Response(null, { status: 204 }))
    expect(await getSamplyProfile('p', true)).toBeNull()
  })
  it('returns expectedInSeconds on 204 with Retry-After', async () => {
    stubFetch(new Response(null, { status: 204, headers: { 'Retry-After': '7' } }))
    expect(await getSamplyProfile('p', true)).toEqual({ expectedInSeconds: 7 })
  })
  it('returns expectedInSeconds 0 on 204 with Retry-After: 0', async () => {
    stubFetch(new Response(null, { status: 204, headers: { 'Retry-After': '0' } }))
    expect(await getSamplyProfile('p', true)).toEqual({ expectedInSeconds: 0 })
  })
  it('throws on 204 with a malformed Retry-After', async () => {
    stubFetch(new Response(null, { status: 204, headers: { 'Retry-After': 'soon' } }))
    await expect(getSamplyProfile('p', true)).rejects.toThrow(/estimated time is not known/)
  })
  it('returns a download handle on 200', async () => {
    stubFetch(new Response(new Uint8Array([1, 2, 3]), { status: 200, headers: {
      'Content-Type': 'application/gzip', 'Content-Disposition': 'attachment; filename="p.json.gz"' } }))
    const result = await getSamplyProfile('p', true)
    expect(result && 'downloadPromise' in result).toBe(true)
  })
})

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mythical-fred-oss

mythical-fred-oss Bot commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

What I ran

Check Result
cargo test -p static-connectors --test connector_suites -- samply_profile (runs the #[engine_test] cases) ✅ 2/2, including the new samply_profile_latest_without_profile
pre-commit run on the changed files (clippy, fmt, OpenAPI/client regen, JS check) ✅
New vitest spec for getSamplyProfile's 204 handling (see inline) ✅ 5/5

Gates: the server change has a test, the changelog marks the incompatible change, and the API docs are fixed in the Rust source. I found no flaky tests, unsafe code or dependency changes.

  • pitfall (medium): merge order for multihost. As the description says, a multihost coordinator that gets a host's 204 without Retry-After treats it as an empty profile. Land the coordinator fix first, or together with this one, so no release ships one without the other.
  • missing (medium): no test covers the web console's new null path. A test for it is in the inline comment.
  • missing (low, existing gap): no test covers latest=true with a profile in progress (204 + Retry-After) or with a stored profile/error (200/500). Both need a samply run, so a follow-up is fine.

Otherwise LGTM.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mihaibudiu
mihaibudiu approved these changes Nov 11, 2026
View reviewed changes

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mythical-fred-oss mythical-fred-oss[bot] mythical-fred-oss[bot] left review comments
@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@blp blp

Labels

API Distributed system APIs bug Something isn't working connectors Issues related to the adapters/connectors crate profiler Issues related to the profiler and its APIs Web Console Related to the browser based UI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@blp @mihaibudiu
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.