Skip to content

Handle 413 with user friendly message#17870

Merged
etiennejouan merged 2 commits intomainfrom
ej/handle-413
Feb 11, 2026
Merged

Handle 413 with user friendly message#17870
etiennejouan merged 2 commits intomainfrom
ej/handle-413

Conversation

@etiennejouan
Copy link
Copy Markdown
Contributor

Add friendly message for 413 errors. Currently, 413 errors originate from the nginx server.

@etiennejouan etiennejouan self-assigned this Feb 11, 2026
@etiennejouan etiennejouan changed the title Handle 413 with friendly message Handle 413 with user friendly message Feb 11, 2026
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 3 files

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts">

<violation number="1" location="packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts:240">
P2: This test can pass without any assertions if makeRequest resolves. Use a rejection assertion (or expect.assertions) so the test fails when the 413 handling isn’t triggered.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

}),
);

try {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Feb 11, 2026

Choose a reason for hiding this comment

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

P2: This test can pass without any assertions if makeRequest resolves. Use a rejection assertion (or expect.assertions) so the test fails when the 413 handling isn’t triggered.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/apollo/services/__tests__/apollo.factory.test.ts, line 240:

<comment>This test can pass without any assertions if makeRequest resolves. Use a rejection assertion (or expect.assertions) so the test fails when the 413 handling isn’t triggered.</comment>

<file context>
@@ -226,4 +228,21 @@ describe('ApolloFactory', () => {
+      }),
+    );
+
+    try {
+      await makeRequest();
+    } catch (error) {
</file context>
Fix with Cubic

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 11, 2026

Greptile Overview

Greptile Summary

This PR adds a user-friendly handling path for HTTP 413 (Payload Too Large) errors coming from the upstream proxy by introducing an onPayloadTooLarge callback in ApolloFactory and wiring it to a deduped error snackbar in useApolloFactory. It also adds a unit test to validate that the callback is invoked on a simulated 413 network error.

The change integrates into the existing Apollo link chain by (1) preventing retries for 413 in RetryLink, and (2) intercepting 413s in errorLink before falling back to the existing debug logging and onNetworkError callback.

Confidence Score: 3/5

  • This PR is close to mergeable but has a runtime-safety issue in retry error handling that should be fixed first.
  • Core 413 handling looks correct and is covered by a unit test, but RetryLink.retryIf calls helpers that assume a ServerError shape; if retryIf receives a non-ServerError value, the code can throw while deciding whether to retry.
  • packages/twenty-front/src/modules/apollo/services/apollo.factory.ts

Important Files Changed

Filename Overview
packages/twenty-front/src/modules/apollo/services/apollo.factory.ts Adds onPayloadTooLarge option and 413 detection in errorLink/retryIf; current retryIf calls statusCode helpers with untyped errors, which can throw at runtime.
packages/twenty-front/src/modules/apollo/hooks/useApolloFactory.ts Wires onPayloadTooLarge callback to enqueue a deduped error snackbar; change is straightforward.
packages/twenty-front/src/modules/apollo/services/tests/apollo.factory.test.ts Adds a test for 413 handling via onPayloadTooLarge callback; test doesn’t assert call count and only asserts substring of message.

Sequence Diagram

sequenceDiagram
  participant UI as Frontend UI
  participant AF as ApolloFactory
  participant RL as RetryLink
  participant EL as ErrorLink
  participant GW as Gateway/Proxy

  UI->>AF: client.mutate/query
  AF->>RL: execute operation
  RL->>GW: HTTP request
  GW-->>RL: 413 Payload Too Large
  RL->>RL: retryIf(error)
  RL-->>EL: propagate networkError
  EL->>AF: isPayloadTooLargeError(networkError)
  AF-->>EL: true
  EL->>UI: onPayloadTooLarge(message)
  UI->>UI: show error snackbar (deduped)
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 11, 2026

Additional Comments (1)

packages/twenty-front/src/modules/apollo/services/apollo.factory.ts
RetryLink can throw
retryIf passes error straight into isAuthenticationError/isPayloadTooLargeError, but those helpers are typed as ServerError and immediately read error.statusCode (apollo.factory.ts:353-359). In practice RetryLink may provide non-ServerError values (e.g. a plain Error), so this is currently relying on an unsafe type mismatch and can crash if error isn’t an object with statusCode.

Fix by guarding/casting before calling, or by making the helpers accept unknown and checking typeof (error as any)?.statusCode === 'number' before comparing.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 11, 2026

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:59954

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Copy Markdown
Member

@FelixMalfait FelixMalfait left a comment

Choose a reason for hiding this comment

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

Great!

@etiennejouan etiennejouan added this pull request to the merge queue Feb 11, 2026
Merged via the queue into main with commit 2f29830 Feb 11, 2026
68 checks passed
@etiennejouan etiennejouan deleted the ej/handle-413 branch February 11, 2026 17:20
@twenty-eng-sync
Copy link
Copy Markdown

Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

1 similar comment
@twenty-eng-sync
Copy link
Copy Markdown

Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants