Skip to content

GeniePlus: fix Guzzle error handling. - #206

Merged
EreMaijala merged 2 commits into
NatLibFi:devfrom
demiankatz:genieplus-guzzle-fix
Aug 4, 2026
Merged

GeniePlus: fix Guzzle error handling.#206
EreMaijala merged 2 commits into
NatLibFi:devfrom
demiankatz:genieplus-guzzle-fix

Conversation

@demiankatz

@demiankatz demiankatz commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

When attempting a GeniePlus reharvest after upgrading to the latest RecordManager, I discovered that OAuth renewals were not working correctly because Laminas HTTP error handling differs from Guzzle HTTP error handling. This PR fixes the problem -- it may be worth reviewing whether any other drivers have similar problems.

TODO

  • Run local test to confirm refactored code still works before merging

@EreMaijala EreMaijala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it would be cleaner to set http_errors to false in $this->httOptions. I did a similar change to SierraApi quite recently, but didn't realize at the time that GeniePlus was similarly affected.

@demiankatz

Copy link
Copy Markdown
Contributor Author

Thanks, @EreMaijala, I agree! My first pass on this was an attempt to use the http_errors setting, but I think I was putting it in the wrong place and it wasn't working. I have revised the code based on your SierraRest example, and I think this should work. I'm running a test now but will have to wait at least 20 minutes to be sure it works. I'll request a re-review now, but please do not merge this until I have confirmed that it actually works on my end!

@demiankatz
demiankatz requested a review from EreMaijala August 3, 2026 15:31
@demiankatz

Copy link
Copy Markdown
Contributor Author

Confirmed: the revised code works the same as the original code.

@EreMaijala
EreMaijala merged commit 0d1fa06 into NatLibFi:dev Aug 4, 2026
4 checks passed
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.

2 participants