Skip to content

Conversation

@djstrong
Copy link
Contributor

@djstrong djstrong commented Mar 6, 2025

@djstrong djstrong added the ensrainbow ENSRainbow related label Mar 6, 2025
@djstrong djstrong self-assigned this Mar 6, 2025
@djstrong djstrong requested a review from a team as a code owner March 6, 2025 11:38
@djstrong djstrong linked an issue Mar 6, 2025 that may be closed by this pull request
@djstrong djstrong added this to ENSNode Mar 6, 2025
@vercel
Copy link

vercel bot commented Mar 6, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
admin.ensnode.io ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2025 2:45pm
ensadmin-next ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2025 2:45pm
ensnode.io ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2025 2:45pm
ensrainbow.io ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 5, 2025 2:45pm

logger.debug(`Heal result:`, result);
return c.json(result, result.errorCode);

// Map error codes > 1000 to 500, otherwise use the original code
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok?

if (
errorMessage.toLowerCase().includes("network") ||
errorMessage.toLowerCase().includes("failed to fetch") ||
errorMessage.toLowerCase().includes("fetch failed")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope it will catch these errors: #214

@djstrong djstrong moved this from In Progress to Ready for Review in ENSNode Mar 10, 2025
return c.json(result, result.errorCode);

// Map error codes > 1000 to 500, otherwise use the original code
const statusCode = result.errorCode && result.errorCode >= 1000 ? 500 : result.errorCode;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can result.errorCode >= 100 be ever true? We haven't changed anything int the server code, so it won't know about new error codes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are new error codes defined:

  // Network error codes
  TIMEOUT: 1000,
  NETWORK_OFFLINE: 1001,
  GENERAL_NETWORK_ERROR: 1099,

@shrugs
Copy link
Collaborator

shrugs commented Apr 2, 2025

next steps: merge main & re-review

Copy link
Collaborator

@shrugs shrugs left a comment

Choose a reason for hiding this comment

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

I'm concerned that the error handling here is slightly overly specified but not worth refactoring — LGTM

@djstrong djstrong requested a review from Copilot April 4, 2025 20:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

Copy link
Contributor

@tk-o tk-o left a comment

Choose a reason for hiding this comment

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

LGTM

@shrugs shrugs self-assigned this Jul 4, 2025
@shrugs shrugs removed their assignment Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ensrainbow ENSRainbow related

Projects

Status: Ready for Review

Development

Successfully merging this pull request may close these issues.

Improve how the ENSRainbow client handles network errors

4 participants