Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: improve POST /relay/v1/auto/messages/{topic} error handling #2339

Merged
merged 21 commits into from
Jan 18, 2024

Conversation

gabrielmer
Copy link
Contributor

@gabrielmer gabrielmer commented Jan 9, 2024

Description

When calling the endpoint POST /relay/v1/auto/messages/{topic} with an invalid content topic, the API response was 200 OK even though it had failed internally in the node.

Added error handling so a proper status code and message get returned.

image

Changes

  • changed return type of publish() from void to Future[Result[void, string]] for subsequent error checking
  • added error handling to POST /relay/v1/auto/messages/{topic} endpoint
  • included test case
  • changed calls to publish() in the codebase to comply with the new function signature

Issue

closes #2190

Copy link

github-actions bot commented Jan 9, 2024

You can find the image built from this PR at

quay.io/wakuorg/nwaku-pr:2339

Built from 989d5d8

@gabrielmer gabrielmer self-assigned this Jan 10, 2024
@gabrielmer gabrielmer force-pushed the chore-improve-rest-api-error-handling-2 branch from 45a3566 to 365ffbb Compare January 16, 2024 13:22
@gabrielmer gabrielmer marked this pull request as ready for review January 16, 2024 17:24
Copy link
Contributor

@SionoiS SionoiS left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

tests/wakunode_rest/test_rest_relay.nim Outdated Show resolved Hide resolved
Copy link
Contributor

@NagyZoltanPeter NagyZoltanPeter left a comment

Choose a reason for hiding this comment

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

Overall LGTM! Thanks for it!

apps/chat2/chat2.nim Outdated Show resolved Hide resolved
@gabrielmer gabrielmer merged commit f841454 into master Jan 18, 2024
9 of 10 checks passed
@gabrielmer gabrielmer deleted the chore-improve-rest-api-error-handling-2 branch January 18, 2024 12:49
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.

bug: lack of error checking in publish
3 participants