Skip to content

enhance request handling with NFData instances#1050

Open
kushagarr wants to merge 8 commits into
brendanhay:mainfrom
kushagarr:feature/force-send-response
Open

enhance request handling with NFData instances#1050
kushagarr wants to merge 8 commits into
brendanhay:mainfrom
kushagarr:feature/force-send-response

Conversation

@kushagarr

@kushagarr kushagarr commented Nov 23, 2025

Copy link
Copy Markdown
Contributor

@endgame this should close #523. Do let me know if you think this is not a correct way of solving the issue

@kushagarr kushagarr marked this pull request as draft November 23, 2025 04:40
@kushagarr kushagarr marked this pull request as ready for review January 14, 2026 06:48
@kushagarr

Copy link
Copy Markdown
Contributor Author

@endgame could you please take a look and let me know if this is moving in right direction ?

@endgame

endgame commented Jan 30, 2026

Copy link
Copy Markdown
Collaborator

I'm not convinced that it's right but I'm also not convinced that it's wrong. For a subtle laziness issue like this we need to replicate it first, and then prove that the PR fixes it.

This probably means standing up some temporary lambda infrastructure to make requests against.

I would probably also consider the strict-wrapper package, but it might be impossible to maintain the interface we currently expose.

@kushagarr

kushagarr commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Here is the gist of the changes: (Generated with AI)

  • Response evaluation is request-specific through AWSRequest.evaluateResponse.
  • The default evaluates only the outer constructor, preserving streaming responses and handwritten instances without NFData.
  • Generated non-streaming operations override the policy with rnf; generated streaming operations retain the shallow default.
  • Evaluation is centralized in Amazonka.HTTP, so signed/unsigned sends, retries, pagination, and waiters behave consistently.
  • The post-hook request is used when selecting the policy.
  • Successful waiter responses are evaluated before acceptors; failed responses are not evaluated.
  • Response hooks run before evaluation.
  • Evaluation exceptions propagate through IO and are not converted to Amazonka.Error.

The handwritten S3 encryption wrappers were also audited:

  • Encrypted a delegates to the wrapped request policy.
  • PutInstructions and DeleteInstructions deeply evaluate their non-streaming responses.
  • Streaming decryption responses remain shallow.

The expanded regression coverage now includes discarded signed and unsigned sends, request hooks, response-hook ordering, retries, pagination, waiter retries, parse failures, streaming body readability, and encrypted request wrappers.

Validation completed:

  • 16 amazonka tests pass.
  • 6 amazonka-s3-encryption tests pass.
  • Fresh S3 and DynamoDB generation and compilation pass.
  • Every DynamoDB operation and every non-streaming S3 operation receives the deep policy.
  • S3 GetObject and GetObjectTorrent retain the streaming-safe shallow policy.
  • Representative S3, DynamoDB, SQS, generator, and encryption builds pass.
  • HLint, Ormolu, and whitespace checks pass.

This keeps the public streaming interface intact while ensuring ordinary parsed responses are fully evaluated at the request boundary, even when callers discard the returned value.

@endgame I would appreciate another look at the final approach.

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.

Call to DynamoDB stalls if response is discarded

2 participants