Skip to content

fix: PGPool error handling and app.js catch/callback bugs#111

Open
arnoldcastro5000 wants to merge 1 commit into
Greenstand:masterfrom
arnoldcastro5000:fix/pgpool-error-handling-app-catch-bugs
Open

fix: PGPool error handling and app.js catch/callback bugs#111
arnoldcastro5000 wants to merge 1 commit into
Greenstand:masterfrom
arnoldcastro5000:fix/pgpool-error-handling-app-catch-bugs

Conversation

@arnoldcastro5000
Copy link
Copy Markdown

@arnoldcastro5000 arnoldcastro5000 commented May 15, 2026

What

Fixes four confirmed bugs in PGPool.js and app.js that cause
permanent request freezes, memory leaks, unhandled process crashes,
and broken 500 error responses.

Fixes Bug 1 in issue: Greenstand/treetracker-infrastructure#315
Related to issue: Greenstand/treetracker-infrastructure#300

Why

PGPool.js

  • A DB error left isFetching[sql] = true permanently, freezing all
    future requests for that SQL string and leaking queued callbacks
  • The getQuery() Promise never rejected on DB error — callers hung
    indefinitely with no error signal

app.js

  • All 4 route catch blocks referenced undefined error instead of
    caught e, causing a ReferenceError that prevented 500 responses
    from being sent
  • throw err inside non-async map.render() and fromString
    callbacks bypassed the outer try/catch, generating unhandled
    exceptions that crash the Node process

Change

PGPool.js

  • Adopted Node.js error-first callback convention cb(err, result)
    on both success and error paths
  • catch block now resets isFetching[sql] and drains the queue,
    calling each pending callback with the error
  • getQuery() consumer updated to handle (err, result) and
    propagate errors via rej(err)

app.js

  • fromString callback: replaced throw "failed" with return rej(err)
  • Both grid routes: renamed _rejrej to bring rejector into scope
  • All render callbacks: replaced throw err with return rej(err)
  • All 4 catch blocks: fixed errore

Follow-up

  • Tests for new error paths in PGPool.js (fetch failure, queue drain)
    and app.js (rej propagation) to be added in a subsequent PR per
    CONTRIBUTING.md testing requirements.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature
  • Breaking change
  • Documentation update

@arnoldcastro5000
Copy link
Copy Markdown
Author

@dadiorchen for your review.

Fixes Bug 1 in issue: Greenstand/treetracker-infrastructure#315

@arnoldcastro5000
Copy link
Copy Markdown
Author

The 2 stuck pods need to be cleared separately. They are not handled by this PR.

PS D:\git\node-mapnik-1> kubectl get pods -n tile-server
NAME READY STATUS RESTARTS AGE
treetracker-tile-server-76c56645c4-7kbfj 1/1 Running 11 (7h3m ago) 22d
treetracker-tile-server-76c56645c4-cn2pt 0/1 ContainerStatusUnknown 6 (39d ago) 41d
treetracker-tile-server-76c56645c4-slzls 1/1 Running 28 (7h3m ago) 36d
treetracker-tile-server-76c56645c4-z8k82 0/1 ContainerStatusUnknown 6 (39d ago) 41d
PS D:\git\node-mapnik-1>

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.

1 participant