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

Fix date check in waittracker #6913

Closed
wants to merge 1 commit into from
Closed

Conversation

stwonary
Copy link
Contributor

This pull request addresses the TypeError issue reported in #6624. The error was caused by attempting to call getTime() on execution.waitTill without verifying if it's a Date object.

@github-actions
Copy link
Contributor

Great PR! Please pay attention to the following items before merging:

Files matching packages/**:

  • If fixing bug, added test to cover scenario.
  • If addressing forum or Github issue, added link to description.

Files matching packages/**/*.ts:

  • Added unit tests to cover new or updated functionality.

Make sure to check off this list before asking for review.

@n8n-assistant n8n-assistant bot added community Authored by a community member core Enhancement outside /nodes-base and /editor-ui labels Aug 10, 2023
@codecov
Copy link

codecov bot commented Aug 10, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (7f0db60) 24.83% compared to head (25a25e5) 24.83%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6913      +/-   ##
==========================================
- Coverage   24.83%   24.83%   -0.01%     
==========================================
  Files        3144     3144              
  Lines      191265   191267       +2     
  Branches    21082    21081       -1     
==========================================
- Hits        47505    47503       -2     
- Misses     142782   142786       +4     
  Partials      978      978              
Files Changed Coverage Δ
packages/cli/src/WaitTracker.ts 0.00% <0.00%> (ø)
...des-base/credentials/DropcontactApi.credentials.ts 0.00% <0.00%> (ø)
...nodes-base/credentials/HighLevelApi.credentials.ts 0.00% <0.00%> (ø)
...des-base/credentials/KoBoToolboxApi.credentials.ts 0.00% <0.00%> (ø)
...s/nodes-base/credentials/NetlifyApi.credentials.ts 0.00% <0.00%> (ø)
...ages/nodes-base/credentials/OdooApi.credentials.ts 0.00% <ø> (ø)
...s/nodes-base/credentials/OnfleetApi.credentials.ts 0.00% <0.00%> (ø)
...redentials/VenafiTlsProtectCloudApi.credentials.ts 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@netroy netroy left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR. But, this doesn't actually solve the issue, it just makes sure that the error isn't unhandled.
This issue was most likely caused by postgres timezone inconsistencies that we have since fixed in #6948

@netroy
Copy link
Member

netroy commented Oct 17, 2023

closing this, and continuing the conversation in #6624 , as I believe this issue doesn't exist in versions after 1.9.0.

@netroy netroy closed this Oct 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member core Enhancement outside /nodes-base and /editor-ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants