Skip to content

Add default timeout support to opal api proxy - #368

Merged
TimDanielsCQI merged 15 commits into
mainfrom
PO-3939
Jul 27, 2026
Merged

Add default timeout support to opal api proxy#368
TimDanielsCQI merged 15 commits into
mainfrom
PO-3939

Conversation

@TimDanielsCQI

@TimDanielsCQI TimDanielsCQI commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Jira link

See PO-3939

Change description

This PR updates opal-frontend-common-node-lib so the shared frontend proxy no longer relies on implicit proxy behaviour for timeout and transport failures. OpalApiProxy now takes an explicit timeout value, passes it through to http-proxy-middleware, and returns a deterministic response when the upstream service times out or cannot be reached.

For timeout and network-style failures, the proxy now returns a 504 Gateway Timeout response with a consistent error body and retriable: true. For other unexpected proxy failures, it returns a 502 Bad Gateway response with retriable: false. The proxy itself does not retry requests, so read and mutation safety stays with the consuming frontend.

I also added a small source-local README under src/proxy/opal-api-proxy/README.md to document the timeout contract, the error mapping, and how the optional retry mechanism in opal-frontend-common-ui-lib can be used by consumers.

Assumptions

  • The upstream service for this proxy is opal-fines-service.
  • A 30 second timeout is the correct default unless a consuming app overrides it.
  • The proxy should expose retryability metadata, but it should not replay requests automatically.
  • Retry behaviour belongs in the frontend consumer, not in the shared proxy library.

Testing

Local Testing Completed

  • Built a local common-node package from the PO-3939 branch and installed it into opal-frontend.
  • Started opal-frontend locally with the PO-3939 tarball.
  • Confirmed http://localhost:4200/sign-in returns 200 OK.
  • Browser smoke test: stopped the local Fines Service container and repeated a safe GET request. The frontend received an immediate 504, confirming the proxy maps a connection/network failure deterministically.
  • The immediate response is expected for a refused connection. The proxy can fail immediately when the upstream is unavailable; it does not need to wait 30 seconds in that case.

Testing Still Outstanding

Some things I couldn't test in the browser and will possibly need integration tests written by QA:

  • A controlled opal-fines-service response delay longer than 30 seconds, to prove the actual proxyTimeout path and confirm a 504 after approximately 30 seconds.
  • End-to-end verification of the fallback 502 mapping for an error outside the recognised timeout/network error-code set.
  • Chrome DevTools can show one browser request, but it cannot alone prove what the server proxy did upstream.

Security Vulnerability Assessment

CVE Suppression: Are there any CVEs present in the codebase (either newly introduced or pre-existing) that are being intentionally suppressed or ignored by this commit?

  • Yes
  • No

Checklist

  • commit messages are meaningful and follow good commit message guidelines
  • README and other documentation has been updated / added (if needed)
  • tests have been updated / new tests has been added (if needed)
  • Does this PR introduce a breaking change

@TimDanielsCQI
TimDanielsCQI marked this pull request as ready for review July 22, 2026 11:34
@TimDanielsCQI TimDanielsCQI self-assigned this Jul 22, 2026

@Arnabsubedi233 Arnabsubedi233 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small one

Comment thread src/proxy/opal-api-proxy/index.ts

@iamfrankiemoran iamfrankiemoran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Remember to bump the library version number

Comment thread src/constants/default-proxy-config.ts Outdated
Comment thread src/proxy/opal-api-proxy/index.ts

@iamfrankiemoran iamfrankiemoran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You need to bump the version number in the package.json

Comment thread src/constants/default-proxy-config.ts Outdated
Comment thread src/interfaces/proxy-config.ts Outdated
Comment thread src/proxy/opal-api-proxy/index.ts
@sonarqubecloud

Copy link
Copy Markdown

@TimDanielsCQI

Copy link
Copy Markdown
Contributor Author

Changes done to this library (including readme) and opal-frontend pr

@iamfrankiemoran iamfrankiemoran left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM ✅

@Arnabsubedi233 Arnabsubedi233 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@TimDanielsCQI
TimDanielsCQI merged commit 9f59545 into main Jul 27, 2026
4 checks passed
@TimDanielsCQI
TimDanielsCQI deleted the PO-3939 branch July 27, 2026 08:51
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