Skip to content

Conversation

@salbahra
Copy link
Member

@salbahra salbahra commented Jun 7, 2025

Summary

  • retry transient ETIMEDOUT errors in OpenGarageApi
  • remove unused dependency in open_garage.js
  • fix default references in tests
  • run mocha via npm test

Testing

  • npm test (fails: Service.OccupancySensor is not a constructor)

https://chatgpt.com/codex/tasks/task_e_684380276384832cb099697ad6529ff7

Copilot AI review requested due to automatic review settings June 7, 2025 12:22
Copy link

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.

Pull Request Overview

This PR enhances timeout handling by retrying transient errors in the API module, cleans up unused dependencies, updates test defaults, and sets up Mocha as the test runner.

  • Added _request wrapper in open_garage_api.js to retry on ECONNRESET and ETIMEDOUT
  • Updated test defaults to use *Secs * 1000 for poll frequency and open/close duration
  • Removed unused request-promise-native import in open_garage.js and changed the npm test script to run Mocha

Reviewed Changes

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

File Description
test/open_garage_test.js Adjusted default timing values to use seconds-based settings × 1000
package.json Updated "test" script to invoke mocha
lib/open_garage_api.js Introduced http.Agent and retry logic for transient timeout errors
lib/open_garage.js Removed unused request-promise-native import comment

let pollFrequencyMs = OpenGarageModule.defaults.pollFrequencyMs
var openDurationMs = OpenGarageModule.defaults.openDurationMs
let pollFrequencyMs = OpenGarageModule.defaults.pollFrequencySecs * 1000
var openDurationMs = OpenGarageModule.defaults.openCloseDurationSecs * 1000
Copy link

Copilot AI Jun 7, 2025

Choose a reason for hiding this comment

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

[nitpick] The declaration uses var while pollFrequencyMs uses let. Consider using let or const for consistency and block scoping.

Suggested change
var openDurationMs = OpenGarageModule.defaults.openCloseDurationSecs * 1000
let openDurationMs = OpenGarageModule.defaults.openCloseDurationSecs * 1000

Copilot uses AI. Check for mistakes.
@@ -1,4 +1,4 @@
const request = require("request-promise-native")
// request-promise-native was previously required here but never used
Copy link

Copilot AI Jun 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Remove this commented-out import to clean up the code and avoid confusion, as it's no longer used.

Suggested change
// request-promise-native was previously required here but never used

Copilot uses AI. Check for mistakes.
if (retries > 0 && err.cause) {
if (err.cause.code === 'ECONNRESET' || err.cause.code === 'ETIMEDOUT') {
log(`Transient ${err.cause.code} error, retrying request`)
return this._request(options, retries - 1)
Copy link

Copilot AI Jun 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Retries occur immediately without delay. Consider adding a short delay (e.g., using setTimeout) between retries to avoid rapid-fire requests.

Suggested change
return this._request(options, retries - 1)
return new Promise((resolve) => {
setTimeout(() => resolve(this._request(options, retries - 1)), 100);
});

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants