Skip to content

fix: golang server replay edge case, response body is empty.#35

Merged
wanlin31 merged 2 commits intogoogle:mainfrom
wanlin31:fix-go
Aug 20, 2025
Merged

fix: golang server replay edge case, response body is empty.#35
wanlin31 merged 2 commits intogoogle:mainfrom
wanlin31:fix-go

Conversation

@wanlin31
Copy link
Copy Markdown
Collaborator

@wanlin31 wanlin31 commented Aug 20, 2025

Minor change to allow the response body to be empty, such as file.create, *.list and *Delete method where the response body is empty.

Update the typescript samples to assert on headers since the response body is empty in samples.

Added a developer guild to for manual e2e testing steps.

Follow up: can add a CI for this e2e test later.

@wanlin31 wanlin31 marked this pull request as ready for review August 20, 2025 21:36
@wanlin31 wanlin31 requested review from Annhiluc, amirh and hkt74 and removed request for amirh August 20, 2025 21:36
@wanlin31 wanlin31 changed the title fix the golang server for edge case: resposne is empty. fix: golang server replay edge case: response body is empty. Aug 20, 2025
@wanlin31 wanlin31 changed the title fix: golang server replay edge case: response body is empty. fix: golang server replay edge case, response body is empty. Aug 20, 2025
@wanlin31 wanlin31 merged commit d059df2 into google:main Aug 20, 2025
2 checks passed

After the previous steps, the directory structure looks like this. The goal is to replace the test-server binary highlighted below, the graph below is a tree view of your directory structure from `test-server/sdks/typescript/samples`.

```diff
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

it is nice to add a detailed structure diagram to help understand the structure, my only concern is we may update the structure in the future and may forget to update this structure.

Copy link
Copy Markdown
Collaborator Author

@wanlin31 wanlin31 Aug 20, 2025

Choose a reason for hiding this comment

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

The reason I want to put it there is, it is really hard to describe which binary to replace, I want to demonstrate that it is the one inside the samples folder would make it effective.

I myself accidentally replaced the ones that in the sdks/typscript/bin/, didn't work at all. I think the better way is to have the CI run this test and we can refer to CI's workflow for manual test.

I will create a internal bug for add the CI and update this documentation.

For now, it serve as a starting point for team members to try the local build. :) Hope that is okay.

}

// When the response body is empty we return directly with the headers.
if len(resp.BodySegments) == 0 {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

wdyt to write the headers? and skip write json bytes

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In the follow up CL, sorry I missed your comment :)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants